From f7c1a4fc930dad17c7b2dad1359dd8ea1a997241 Mon Sep 17 00:00:00 2001 From: Michael Jolley Date: Wed, 29 Jul 2026 12:21:49 -0500 Subject: [PATCH] [CmdPal] JS/TS Extensions Phase 4: extension service, wrapper, discovery + host wiring Adds the JSON-RPC-backed extension service that launches, supervises, and disposes JS/TS Command Palette extensions, wires host status notifications, extension discovery, and the install root under the CmdPal data folder. Hardens the service lifecycle, file watchers, node.exe resolution, and JS numeric color/int parsing; publisher falls back to the npm author. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: d243b1e9-40fb-4aed-aa60-beb5e80f7d91 --- .../Models/JSCommandProviderProxy.cs | 469 +++-- .../Models/JSListItemAdapter.cs | 37 + .../Models/JSListPageProxy.cs | 76 +- .../Models/JSObservableProxyBase.cs | 18 + .../CommandProviderWrapper.cs | 45 + .../Models/JSExtensionWrapper.cs | 836 ++++++++ .../Services/DirectoryLifecycleGate.cs | 221 ++ .../Services/HotReloadDebouncer.cs | 180 ++ .../Services/JsonRpcExtensionService.cs | 1798 +++++++++++++++++ .../Services/NodeRuntimeLocator.cs | 71 + .../Services/ProviderIdReservations.cs | 95 + .../Services/ReloadCancellation.cs | 129 ++ .../Services/SerialNotificationDispatcher.cs | 105 + .../cmdpal/Microsoft.CmdPal.UI/App.xaml.cs | 1 + .../DirectoryLifecycleGateTests.cs | 206 ++ .../HotReloadDebouncerTests.cs | 136 ++ .../JSCommandProviderProxyFrozenTests.cs | 78 + ...ndProviderProxyStartupNotificationTests.cs | 91 + ...mandProviderProxyStatusDisposeRaceTests.cs | 95 + ...ommandProviderProxyStatusLifecycleTests.cs | 122 ++ .../JSExtensionWrapperBootstrapTests.cs | 126 ++ .../JSExtensionWrapperTests.cs | 123 ++ .../JSListItemAdapterKeyTests.cs | 62 + ...onRpcExtensionServiceCrashRecoveryTests.cs | 68 + .../JsonRpcExtensionServiceDiscoveryTests.cs | 149 ++ ...nRpcExtensionServiceReconciliationTests.cs | 202 ++ ...nRpcExtensionServiceWatcherRoutingTests.cs | 235 +++ .../NodeRuntimeLocatorTests.cs | 75 + .../ProviderIdReservationsTests.cs | 132 ++ .../RecordingExtensionHost.cs | 139 ++ .../ReloadCancellationTests.cs | 66 + .../SerialNotificationDispatcherTests.cs | 129 ++ 32 files changed, 6145 insertions(+), 170 deletions(-) create mode 100644 src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Models/JSExtensionWrapper.cs create mode 100644 src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Services/DirectoryLifecycleGate.cs create mode 100644 src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Services/HotReloadDebouncer.cs create mode 100644 src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Services/JsonRpcExtensionService.cs create mode 100644 src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Services/NodeRuntimeLocator.cs create mode 100644 src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Services/ProviderIdReservations.cs create mode 100644 src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Services/ReloadCancellation.cs create mode 100644 src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Services/SerialNotificationDispatcher.cs create mode 100644 src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/DirectoryLifecycleGateTests.cs create mode 100644 src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/HotReloadDebouncerTests.cs create mode 100644 src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JSCommandProviderProxyFrozenTests.cs create mode 100644 src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JSCommandProviderProxyStartupNotificationTests.cs create mode 100644 src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JSCommandProviderProxyStatusDisposeRaceTests.cs create mode 100644 src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JSCommandProviderProxyStatusLifecycleTests.cs create mode 100644 src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JSExtensionWrapperBootstrapTests.cs create mode 100644 src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JSExtensionWrapperTests.cs create mode 100644 src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JSListItemAdapterKeyTests.cs create mode 100644 src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JsonRpcExtensionServiceCrashRecoveryTests.cs create mode 100644 src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JsonRpcExtensionServiceDiscoveryTests.cs create mode 100644 src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JsonRpcExtensionServiceReconciliationTests.cs create mode 100644 src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JsonRpcExtensionServiceWatcherRoutingTests.cs create mode 100644 src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/NodeRuntimeLocatorTests.cs create mode 100644 src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/ProviderIdReservationsTests.cs create mode 100644 src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/RecordingExtensionHost.cs create mode 100644 src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/ReloadCancellationTests.cs create mode 100644 src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/SerialNotificationDispatcherTests.cs diff --git a/src/modules/cmdpal/Microsoft.CmdPal.JsonRpc/Models/JSCommandProviderProxy.cs b/src/modules/cmdpal/Microsoft.CmdPal.JsonRpc/Models/JSCommandProviderProxy.cs index 4b3cdd19ff..7dbc645ba1 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.JsonRpc/Models/JSCommandProviderProxy.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.JsonRpc/Models/JSCommandProviderProxy.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Text.Json; using System.Text.Json.Nodes; @@ -23,26 +24,38 @@ namespace Microsoft.CmdPal.JsonRpc.Models; public sealed partial class JSCommandProviderProxy : ICommandProvider4, IDisposable { private readonly JsonRpcConnection _connection; - private readonly JsonElement _providerMetadata; - private readonly string _id; - private readonly string _displayName; - private readonly IIconInfo _icon; + private readonly string _fallbackId; + private readonly string _fallbackDisplayName; + private readonly string? _fallbackIcon; + + // Guards the provider metadata, which is set once from the initialize handshake + // after the proxy is constructed (the proxy is created before the handshake so its + // notification handlers are registered in time to receive startup notifications). + private readonly Lock _metadataLock = new(); // Host status messages use the statusId minted by the client. That lets an // update refresh the same message and lets hide target the right one. private readonly Dictionary _shownStatusMessages = new(); + private readonly ConcurrentDictionary _fallbackAdapters = new(); - // Guards _shownStatusMessages. Host status notifications and Dispose can run - // on different threads, so reads, writes, and enumeration share one gate. - private readonly object _statusLock = new(); + // Guards the host reference, the shown-status bookkeeping, and the buffer of host + // actions emitted before the host is attached. Notifications an extension raises + // while it activates (during the initialize handshake) arrive before the host is + // set; they are buffered here and flushed in order once the host attaches so those + // startup logs, statuses, and clipboard requests are not lost. + private readonly Lock _hostLock = new(); + private readonly List> _pendingHostActions = []; + + private JsonElement _providerMetadata; + + // Provider identity carried from the initialize handshake metadata. Each field + // falls back to the configured values when the handshake omits it. Guarded by _metadataLock + // because SetProviderMetadata can refresh them after the proxy is already in use. + private string _id = "unknown"; + private string _displayName = string.Empty; + private IIconInfo _icon = new IconInfo(string.Empty); private readonly object _settingsLock = new(); - // Host notifications can arrive after this proxy subscribes but before - // InitializeWithHost attaches the host. Buffer them in arrival order so - // startup status and log messages are not dropped. A null buffer means the - // host is attached and notifications can run inline. - private readonly object _preInitLock = new(); - private List? _preInitNotifications = new(); private IExtensionHost? _host; private ICommandSettings? _settingsCache; private bool _settingsLoading; @@ -59,28 +72,76 @@ public sealed partial class JSCommandProviderProxy : ICommandProvider4, IDisposa _connection = connection ?? throw new ArgumentNullException(nameof(connection)); ArgumentException.ThrowIfNullOrEmpty(fallbackId); ArgumentNullException.ThrowIfNull(fallbackDisplayName); + _fallbackId = fallbackId; + _fallbackDisplayName = fallbackDisplayName; + _fallbackIcon = fallbackIcon; _providerMetadata = providerMetadata; - // Prefer the identity and icon from the initialize handshake when they are - // present. If the extension omits a field, use the package manifest value. - _id = ReadHandshakeString(providerMetadata, "id") ?? fallbackId; - _displayName = ReadHandshakeString(providerMetadata, "displayName") ?? fallbackDisplayName; - _icon = ReadHandshakeIcon(providerMetadata) ?? new IconInfo(fallbackIcon ?? string.Empty); + // Apply the identity captured at construction. The proxy is created before the + // initialize handshake completes, so this seeds id, display name, and icon from + // whatever metadata is available now (or the configured fallback); SetProviderMetadata later + // refreshes them with the real handshake values. + lock (_metadataLock) + { + ApplyProviderIdentityLocked(providerMetadata); + } RegisterNotificationHandlers(); + + // Clean up any active host statuses when the extension disconnects or its process + // exits, not only when it explicitly hides them. The wrapper also disposes this + // proxy during teardown; both paths are idempotent. + _connection.Disconnected += OnConnectionDisconnected; } public event TypedEventHandler? ItemsChanged; - public string Id => _id; + public string Id + { + get + { + lock (_metadataLock) + { + return _id; + } + } + } - public string DisplayName => _displayName; + public string DisplayName + { + get + { + lock (_metadataLock) + { + return _displayName; + } + } + } - public IIconInfo Icon => _icon; + public IIconInfo Icon + { + get + { + lock (_metadataLock) + { + return _icon; + } + } + } - // True means the provider's top-level command set is fixed. If the extension - // leaves it out of the handshake, the wire default is true. - public bool Frozen => ReadFrozen(_providerMetadata); + // Whether the provider's top-level command set is fixed. The value is carried + // from the initialize handshake metadata; the wire default is true when the + // extension does not specify it. + public bool Frozen + { + get + { + lock (_metadataLock) + { + return ReadFrozen(_providerMetadata); + } + } + } public ICommandSettings? Settings { @@ -240,76 +301,181 @@ public sealed partial class JSCommandProviderProxy : ICommandProvider4, IDisposa { ArgumentNullException.ThrowIfNull(host); - List buffered; - lock (_preInitLock) + lock (_hostLock) { + if (_isDisposed) + { + return; + } + _host = host; - // Swap the buffer to null under the same lock used by notification handlers. - // _host is written first so any handler that now runs inline sees the host, - // not a stale null. - buffered = _preInitNotifications ?? new List(); - _preInitNotifications = null; + // Deliver, in arrival order, the host actions that the extension emitted while + // it was activating (before the host was attached), such as startup logs, + // statuses, and clipboard requests. Replaying under the lock keeps this delivery + // ordered against a concurrent dispose or disconnect so a buffered show cannot + // land after teardown has already hidden everything. + foreach (var action in _pendingHostActions) + { + try + { + action(host); + } + catch (Exception ex) + { + Logger.LogWarning($"Error flushing buffered host action for {DisplayName}: {ex.Message}"); + } + } + + _pendingHostActions.Clear(); } Logger.LogDebug($"JSCommandProviderProxy initialized with host for {DisplayName}"); + } - // Replay startup notifications in arrival order now that the host can receive them. - foreach (var notification in buffered) + /// + /// Sets the provider metadata captured from the initialize handshake so that the + /// author-specified value and the handshake identity (id, + /// display name, and icon) flow through instead of the wire defaults. Called by the + /// extension wrapper after the handshake completes. + /// + /// The provider metadata returned during initialize. + internal void SetProviderMetadata(JsonElement providerMetadata) + { + lock (_metadataLock) { - DispatchBufferedHostNotification(notification.Method, notification.Parameters); + _providerMetadata = providerMetadata; + ApplyProviderIdentityLocked(providerMetadata); } } - public void Dispose() + /// + /// Runs a host action now when the host is attached, or buffers it in arrival order to + /// be replayed once attaches the host. This keeps + /// notifications emitted during activation (before the host is set) from being dropped. + /// + private void RunWithHost(Action action) + { + lock (_hostLock) + { + RunWithHostLocked(action); + } + } + + /// + /// Variant of that requires to already + /// be held by the caller. Status show and hide handlers call this from inside the same + /// lock acquisition that mutates so the dictionary + /// update and the host dispatch are a single atomic, ordered step. Splitting them across + /// two lock acquisitions would let a hide dispatch overtake its pending show. + /// + private void RunWithHostLocked(Action action) { if (_isDisposed) { return; } - _isDisposed = true; + var host = _host; + if (host is null) + { + _pendingHostActions.Add(action); + return; + } - // Detach this proxy's handlers so late connection notifications stop here. - // The extension service owns process teardown and protocol dispose, so this - // proxy only releases its subscriptions and host references. + // Invoke the host action while holding the lock so status show and hide calls + // run in the same order as their lock acquisition. Host status methods are + // fire-and-forget (they return an async operation immediately), so the lock is + // held only briefly. This keeps a hide from being reordered ahead of a late + // show, and, because Dispose sets _isDisposed under this same lock, keeps a show + // from resurrecting status after teardown has hidden everything. + action(host); + } + + public void Dispose() + { + lock (_hostLock) + { + if (_isDisposed) + { + return; + } + + _isDisposed = true; + + var host = _host; + _host = null; + _pendingHostActions.Clear(); + var activeStatuses = new List(_shownStatusMessages.Values); + _shownStatusMessages.Clear(); + + // Hide any status still visible while holding the lock so the hide is ordered + // after every show already dispatched and cannot be overtaken by a late show. + // Once _isDisposed is set here, RunWithHost is a no-op, so no show can resurrect + // status after teardown has hidden it. + HideStatuses(host, activeStatuses); + } + + _connection.Disconnected -= OnConnectionDisconnected; + + // Detach every notification handler this proxy registered so late + // notifications from the connection are no longer routed here. Process + // teardown and the protocol dispose request are owned by the extension + // service, so this proxy only releases its own subscriptions and host + // references. See the W4 coordination note in the remediation report. foreach (var method in RegisteredNotificationMethods) { _connection.UnregisterNotificationHandler(method); } - // Hide any status messages still on screen. Snapshot and clear under the lock - // so a status notification racing Dispose cannot change the map during enumeration. - var host = _host; - List pendingStatuses; - lock (_statusLock) - { - pendingStatuses = new List(_shownStatusMessages.Values); - _shownStatusMessages.Clear(); - } - - if (host != null) - { - foreach (var status in pendingStatuses) - { - try - { - _ = host.HideStatus(status); - } - catch (Exception ex) - { - Logger.LogWarning($"Error hiding status during dispose for {DisplayName}: {ex.Message}"); - } - } - } - lock (_settingsLock) { (_settingsCache as IDisposable)?.Dispose(); Monitor.PulseAll(_settingsLock); } + } - _host = null; + private void OnConnectionDisconnected(object? sender, EventArgs e) + { + // The extension disconnected or its process exited. Clear any active statuses so + // they do not linger in the host UI even though no explicit hide arrived. The + // notification handlers stay registered; the connection is gone so they cannot + // fire again, and Dispose still unregisters them during full teardown. + lock (_hostLock) + { + if (_isDisposed || _shownStatusMessages.Count == 0) + { + return; + } + + var host = _host; + var activeStatuses = new List(_shownStatusMessages.Values); + _shownStatusMessages.Clear(); + + // Hide under the lock so this teardown hide is ordered against any concurrent + // show or dispose and cannot strand or resurrect status. + HideStatuses(host, activeStatuses); + } + } + + private void HideStatuses(IExtensionHost? host, List statuses) + { + if (host is null) + { + return; + } + + foreach (var status in statuses) + { + try + { + _ = host.HideStatus(status); + } + catch (Exception ex) + { + Logger.LogWarning($"Error hiding status for {DisplayName}: {ex.Message}"); + } + } } private static readonly string[] RegisteredNotificationMethods = @@ -360,40 +526,6 @@ public sealed partial class JSCommandProviderProxy : ICommandProvider4, IDisposa } } - // Buffers a host notification until InitializeWithHost attaches the host. - // The params element is cloned because the connection may recycle the source document. - private bool TryBufferUntilHostAttached(string method, JsonElement paramsElement) - { - lock (_preInitLock) - { - if (_preInitNotifications == null) - { - return false; - } - - _preInitNotifications.Add(new BufferedHostNotification(method, paramsElement.Clone())); - return true; - } - } - - // Runs the buffered notification now that the host is attached. The open gate - // keeps this pass from buffering the same notification again. - private void DispatchBufferedHostNotification(string method, JsonElement paramsElement) - { - switch (method) - { - case "host/showStatus": - HandleShowStatusNotification(paramsElement); - break; - case "host/hideStatus": - HandleHideStatusNotification(paramsElement); - break; - case "host/logMessage": - HandleLogMessageNotification(paramsElement); - break; - } - } - private void HandleItemsChangedNotification(JsonElement paramsElement) { if (_isDisposed) @@ -429,6 +561,23 @@ public sealed partial class JSCommandProviderProxy : ICommandProvider4, IDisposa try { JSPropertyChangeRegistry.Dispatch(_connection, paramsElement); + + var commandId = JSModelMapper.GetString(paramsElement, "commandId") ?? string.Empty; + if (string.IsNullOrEmpty(commandId) || + !_fallbackAdapters.TryGetValue(commandId, out var fallbackAdapter)) + { + return; + } + + if (paramsElement.TryGetProperty("properties", out var propsProp) && + propsProp.ValueKind == JsonValueKind.Object) + { + var displayTitle = JSModelMapper.GetString(propsProp, "displayTitle"); + if (displayTitle != null) + { + fallbackAdapter.UpdateDisplayTitle(displayTitle); + } + } } catch (Exception ex) { @@ -443,11 +592,6 @@ public sealed partial class JSCommandProviderProxy : ICommandProvider4, IDisposa return; } - if (TryBufferUntilHostAttached("host/logMessage", paramsElement)) - { - return; - } - try { var message = JSModelMapper.GetString(paramsElement, "message"); @@ -476,11 +620,8 @@ public sealed partial class JSCommandProviderProxy : ICommandProvider4, IDisposa break; } - if (_host != null) - { - var logMessage = new LogMessage { Message = message, State = (MessageState)state }; - _ = _host.LogMessage(logMessage); - } + var logMessage = new LogMessage { Message = message, State = (MessageState)state }; + RunWithHost(host => _ = host.LogMessage(logMessage)); } catch (Exception ex) { @@ -495,11 +636,6 @@ public sealed partial class JSCommandProviderProxy : ICommandProvider4, IDisposa return; } - if (TryBufferUntilHostAttached("host/showStatus", paramsElement)) - { - return; - } - try { var (message, state) = ReadStatusMessage(paramsElement); @@ -510,32 +646,29 @@ public sealed partial class JSCommandProviderProxy : ICommandProvider4, IDisposa var statusId = ReadStatusId(paramsElement); var progress = ReadProgress(paramsElement); + var context = ReadStatusContext(paramsElement); - lock (_statusLock) + StatusMessage statusMessage; + lock (_hostLock) { if (_isDisposed) { return; } - var host = _host; - if (host == null) - { - return; - } - if (!string.IsNullOrEmpty(statusId) && _shownStatusMessages.TryGetValue(statusId, out var existing)) { - // Same statusId again. Update the existing message instead of - // stacking another one. + // Same status shown again: refresh it in place so the host keeps a + // single message rather than stacking duplicates. The buffered or + // already-delivered ShowStatus references this same object. existing.Message = message; existing.State = (MessageState)state; existing.Progress = progress; return; } - var statusMessage = new StatusMessage + statusMessage = new StatusMessage { Message = message, State = (MessageState)state, @@ -547,12 +680,9 @@ public sealed partial class JSCommandProviderProxy : ICommandProvider4, IDisposa _shownStatusMessages[statusId] = statusMessage; } - // Keep the map update and ShowStatus under one lock. Dispose uses - // this same lock to hide tracked statuses, so it either runs before - // this show starts or after the shown status is tracked. Releasing - // the lock between those steps would let Dispose hide a status that - // was not shown yet. - _ = host.ShowStatus(statusMessage, ReadStatusContext(paramsElement)); + // Dispatch inside the same lock acquisition that recorded the status so the + // show cannot be reordered behind a hide that arrives immediately after. + RunWithHostLocked(host => _ = host.ShowStatus(statusMessage, context)); } } catch (Exception ex) @@ -568,33 +698,32 @@ public sealed partial class JSCommandProviderProxy : ICommandProvider4, IDisposa return; } - if (TryBufferUntilHostAttached("host/hideStatus", paramsElement)) - { - return; - } - try { - var host = _host; - if (host == null) + var statusId = ReadStatusId(paramsElement); + if (string.IsNullOrEmpty(statusId)) { return; } - var statusId = ReadStatusId(paramsElement); - StatusMessage statusMessage; - lock (_statusLock) + lock (_hostLock) { - if (string.IsNullOrEmpty(statusId) || - !_shownStatusMessages.TryGetValue(statusId, out statusMessage!)) + if (_isDisposed) + { + return; + } + + if (!_shownStatusMessages.TryGetValue(statusId, out var existing)) { return; } _shownStatusMessages.Remove(statusId); - } - _ = host.HideStatus(statusMessage); + // Dispatch inside the same lock acquisition that removed the status so the + // hide observes the same ordering as the show that preceded it. + RunWithHostLocked(host => _ = host.HideStatus(existing)); + } } catch (Exception ex) { @@ -626,7 +755,7 @@ public sealed partial class JSCommandProviderProxy : ICommandProvider4, IDisposa private static int ReadState(JsonElement paramsElement) { if (paramsElement.ValueKind == JsonValueKind.Object && - JSModelMapper.TryGetProperty(paramsElement, "state", out var stateProp) && + JSModelMapper.TryGetAnyCase(paramsElement, "state", "State", out var stateProp) && stateProp.ValueKind == JsonValueKind.Number) { return stateProp.GetInt32(); @@ -638,7 +767,7 @@ public sealed partial class JSCommandProviderProxy : ICommandProvider4, IDisposa private static string ReadStatusId(JsonElement paramsElement) { if (paramsElement.ValueKind == JsonValueKind.Object && - JSModelMapper.TryGetProperty(paramsElement, "statusId", out var idProp) && + JSModelMapper.TryGetAnyCase(paramsElement, "statusId", "StatusId", out var idProp) && idProp.ValueKind == JsonValueKind.String) { return idProp.GetString() ?? string.Empty; @@ -647,8 +776,8 @@ public sealed partial class JSCommandProviderProxy : ICommandProvider4, IDisposa return string.Empty; } - // Turns the wire progress payload into the toolkit shape. Null means no - // progress was reported. + // Maps a status progress payload (indeterminate spinner or a percentage) onto + // a toolkit progress state. Returns null when no progress is reported. private static IProgressState? ReadProgress(JsonElement paramsElement) { if (paramsElement.ValueKind != JsonValueKind.Object || @@ -658,24 +787,36 @@ public sealed partial class JSCommandProviderProxy : ICommandProvider4, IDisposa return null; } - var progress = new ProgressState(); + var isIndeterminate = + JSModelMapper.TryGetProperty(progressProp, "isIndeterminate", out var indeterminateProp) && + indeterminateProp.ValueKind == JsonValueKind.True; - if (JSModelMapper.TryGetProperty(progressProp, "isIndeterminate", out var indeterminateProp)) - { - progress.IsIndeterminate = indeterminateProp.ValueKind == JsonValueKind.True; - } + var progress = new ProgressState { IsIndeterminate = isIndeterminate }; if (JSModelMapper.TryGetProperty(progressProp, "progressPercent", out var percentProp) && percentProp.ValueKind == JsonValueKind.Number && - percentProp.TryGetUInt32(out var percent)) + percentProp.TryGetDouble(out var percent) && + percent >= 0) { - progress.ProgressPercent = percent; + progress.ProgressPercent = percent >= uint.MaxValue ? uint.MaxValue : (uint)percent; } return progress; } - // Blank or missing handshake fields use the configured fallback value. + // Caller must hold _metadataLock. Applies the provider identity (id, display name, + // and icon) from the initialize handshake metadata, falling back to configured values for + // any field the handshake omits. Called from the constructor with the metadata passed + // at construction and again from SetProviderMetadata when the real handshake completes. + private void ApplyProviderIdentityLocked(JsonElement metadata) + { + _id = ReadHandshakeString(metadata, "id") ?? _fallbackId; + _displayName = ReadHandshakeString(metadata, "displayName") ?? _fallbackDisplayName; + _icon = ReadHandshakeIcon(metadata) ?? new IconInfo(_fallbackIcon ?? string.Empty); + } + + // Reads a string field declared in the initialize handshake metadata. Returns null when absent or empty so the caller + // falls back to the manifest value rather than overwriting it with an empty string. private static string? ReadHandshakeString(JsonElement metadata, string name) { if (metadata.ValueKind == JsonValueKind.Object && @@ -689,7 +830,9 @@ public sealed partial class JSCommandProviderProxy : ICommandProvider4, IDisposa return null; } - // Missing handshake icons fall back to the manifest icon, not an empty glyph. + // Reads the icon declared in the initialize handshake metadata. Returns null + // when the handshake omits an icon so the caller falls back to the manifest + // icon rather than replacing it with an empty glyph. private static IIconInfo? ReadHandshakeIcon(JsonElement metadata) { if (metadata.ValueKind == JsonValueKind.Object && @@ -717,7 +860,7 @@ public sealed partial class JSCommandProviderProxy : ICommandProvider4, IDisposa } } - // The wire default is frozen when the extension leaves the flag out. + // The wire default when the extension omits the flag is frozen. return true; } @@ -796,12 +939,14 @@ public sealed partial class JSCommandProviderProxy : ICommandProvider4, IDisposa { var adapter = new JSFallbackCommandItemAdapter(element, _connection); items.Add(adapter); + + var id = adapter.Id; + if (!string.IsNullOrEmpty(id)) + { + _fallbackAdapters[id] = adapter; + } } return items.ToArray(); } - - // An arrival-ordered snapshot of a host notification that reached this proxy - // before the host was attached, held until InitializeWithHost replays it. - private readonly record struct BufferedHostNotification(string Method, JsonElement Parameters); } diff --git a/src/modules/cmdpal/Microsoft.CmdPal.JsonRpc/Models/JSListItemAdapter.cs b/src/modules/cmdpal/Microsoft.CmdPal.JsonRpc/Models/JSListItemAdapter.cs index 9650c061d1..b82dfaa68e 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.JsonRpc/Models/JSListItemAdapter.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.JsonRpc/Models/JSListItemAdapter.cs @@ -22,6 +22,18 @@ internal sealed partial class JSListItemAdapter : JSObservableProxyBase, IListIt private readonly JSLazyCache _command; private readonly JSLazyCache _moreCommands; private readonly JSLazyCache _details; + private static readonly string[] RefreshableProperties = + [ + "command", + "moreCommands", + "icon", + "title", + "subtitle", + "tags", + "details", + "section", + "textToSuggest", + ]; public JSListItemAdapter(JsonElement data, JsonRpcConnection connection) : base(GetNotificationId(data), connection, data) @@ -83,6 +95,31 @@ internal sealed partial class JSListItemAdapter : JSObservableProxyBase, IListIt } } + internal static string ComputeKey(JsonElement data) + { + var id = JSModelMapper.GetString(data, "id"); + if (!string.IsNullOrEmpty(id)) + { + return "id:" + id; + } + + if (JSModelMapper.TryGetCommandData(data, out var commandData)) + { + var commandId = JSModelMapper.GetString(commandData, "id"); + if (!string.IsNullOrEmpty(commandId)) + { + return "cmd:" + commandId; + } + } + + return "title:" + (JSModelMapper.GetString(data, "title") ?? string.Empty); + } + + internal void UpdateData(JsonElement data) + { + ReplaceData(data, RefreshableProperties); + } + public override void Dispose() { _moreCommands.Dispose(); diff --git a/src/modules/cmdpal/Microsoft.CmdPal.JsonRpc/Models/JSListPageProxy.cs b/src/modules/cmdpal/Microsoft.CmdPal.JsonRpc/Models/JSListPageProxy.cs index e635069c87..967da71d88 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.JsonRpc/Models/JSListPageProxy.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.JsonRpc/Models/JSListPageProxy.cs @@ -36,9 +36,17 @@ internal sealed partial class JSListPageProxy : JSObservableProxyBase, IListPage private readonly object _stateLock = new(); private readonly JSLazyCache _filters; private readonly JSLazyCache _emptyContent; + private readonly object _itemCacheLock = new(); private bool? _hasMoreItemsState; private bool _disposed; + // Adapters from the previous GetItems call, keyed by stable identity. Reusing + // the same IListItem instance for an item that persists across a refresh lets + // the host's reference-keyed view model cache keep the existing view model, + // which preserves the current list selection when a dynamic page rebuilds its + // items. A queue per key keeps reuse stable when several items share a title. + private Dictionary> _adapterCache = new(StringComparer.Ordinal); + public JSListPageProxy(string pageId, JsonRpcConnection connection, JsonElement pageData = default) : base(pageId, connection, pageData) { @@ -288,6 +296,7 @@ internal sealed partial class JSListPageProxy : JSObservableProxyBase, IListPage _filters.Dispose(); _emptyContent.Dispose(); + ResetAdapterCache(); base.Dispose(); _registry.Pages.Unregister(_pageId, this); @@ -386,6 +395,7 @@ internal sealed partial class JSListPageProxy : JSObservableProxyBase, IListPage { if (!result.HasValue) { + ResetAdapterCache(); return []; } @@ -398,26 +408,76 @@ internal sealed partial class JSListPageProxy : JSObservableProxyBase, IListPage if (arrayElement.ValueKind != JsonValueKind.Array) { + ResetAdapterCache(); return []; } var items = new List(); - foreach (var element in arrayElement.EnumerateArray()) + + lock (_itemCacheLock) { - if (element.ValueKind == JsonValueKind.Object && - JSModelMapper.GetBool(element, "_isSeparator", false)) + var previousCache = _adapterCache; + var nextCache = new Dictionary>(StringComparer.Ordinal); + + foreach (var element in arrayElement.EnumerateArray()) { - items.Add(new Separator(JSModelMapper.GetString(element, "title") ?? string.Empty)); - } - else - { - items.Add(new JSListItemAdapter(element, Connection)); + if (element.ValueKind == JsonValueKind.Object && + JSModelMapper.GetBool(element, "_isSeparator", false)) + { + items.Add(new Separator(JSModelMapper.GetString(element, "title") ?? string.Empty)); + continue; + } + + var key = JSListItemAdapter.ComputeKey(element); + JSListItemAdapter adapter; + if (previousCache.TryGetValue(key, out var previousQueue) && previousQueue.Count > 0) + { + adapter = previousQueue.Dequeue(); + adapter.UpdateData(element); + } + else + { + adapter = new JSListItemAdapter(element, Connection); + } + + items.Add(adapter); + + if (!nextCache.TryGetValue(key, out var nextQueue)) + { + nextQueue = new Queue(); + nextCache[key] = nextQueue; + } + + nextQueue.Enqueue(adapter); } + + DisposeAdapters(previousCache); + _adapterCache = nextCache; } return items.ToArray(); } + private void ResetAdapterCache() + { + lock (_itemCacheLock) + { + DisposeAdapters(_adapterCache); + _adapterCache = new Dictionary>(StringComparer.Ordinal); + } + } + + private static void DisposeAdapters(Dictionary> cache) + { + foreach (var adapters in cache.Values) + { + while (adapters.TryDequeue(out var adapter)) + { + adapter.Dispose(); + } + } + } + private sealed class PageRegistry { private readonly object _subscribeLock = new(); diff --git a/src/modules/cmdpal/Microsoft.CmdPal.JsonRpc/Models/JSObservableProxyBase.cs b/src/modules/cmdpal/Microsoft.CmdPal.JsonRpc/Models/JSObservableProxyBase.cs index d14b657d66..ecb5b16d51 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.JsonRpc/Models/JSObservableProxyBase.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.JsonRpc/Models/JSObservableProxyBase.cs @@ -79,6 +79,24 @@ internal abstract class JSObservableProxyBase : BaseObservable, IJSPropertyChang { } + protected void ReplaceData(JsonElement data, IReadOnlyList propertyNames) + { + var current = Data; + if (current.ValueKind != JsonValueKind.Undefined && + string.Equals(current.GetRawText(), data.GetRawText(), StringComparison.Ordinal)) + { + return; + } + + Volatile.Write(ref _data, new DataBox(data)); + OnPropertyChangesApplied(propertyNames); + + foreach (var propertyName in propertyNames) + { + OnPropertyChanged(ToAbiPropertyName(propertyName)); + } + } + public virtual void Dispose() { if (_disposed) diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/CommandProviderWrapper.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/CommandProviderWrapper.cs index 4a3250d8da..f2c307699b 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/CommandProviderWrapper.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/CommandProviderWrapper.cs @@ -125,6 +125,51 @@ public sealed class CommandProviderWrapper : ICommandProviderContext isValid = true; } + /// + /// Creates a wrapper for a JavaScript extension where the + /// is obtained directly over JSON-RPC (not through ). + /// + /// The JS extension wrapper managing the Node.js process. + /// The command provider proxy backed by the JSON-RPC connection. + /// The UI thread scheduler. + public CommandProviderWrapper(IExtensionWrapper extension, ICommandProvider provider, TaskScheduler mainThread) + { + _taskScheduler = mainThread; + TopLevelPageContext = new(this, _taskScheduler); + + Extension = extension; + ExtensionHost = new CommandPaletteHost(extension); + _commandProvider = new(provider); + + try + { + var model = _commandProvider.Unsafe!; + + // Hook the extension back into us + model.InitializeWithHost(ExtensionHost); + model.ItemsChanged += CommandProvider_ItemsChanged; + + Id = provider.Id; + DisplayName = provider.DisplayName; + Icon = new(provider.Icon); + Icon.InitializeProperties(); + + // Note: explicitly not InitializeProperties()ing the settings here. If + // we do that, then we'd regress GH #38321 + Settings = new(provider.Settings, this, _taskScheduler); + + isValid = true; + + Logger.LogDebug($"Initialized JS extension command provider {Extension.PackageFamilyName}:{Extension.ExtensionUniqueId}"); + } + catch (Exception e) + { + Logger.LogError("Failed to initialize CommandProvider for JS extension."); + Logger.LogError($"Extension was {Extension!.PackageFamilyName}"); + Logger.LogError(e.ToString()); + } + } + private ProviderSettings GetProviderSettings(SettingsModel settings) { if (!settings.ProviderSettings.TryGetValue(ProviderId, out var ps)) diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Models/JSExtensionWrapper.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Models/JSExtensionWrapper.cs new file mode 100644 index 0000000000..44ac6ce2cc --- /dev/null +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Models/JSExtensionWrapper.cs @@ -0,0 +1,836 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.IO; +using System.Security.Cryptography; +using System.Text; +using System.Text.Json; +using System.Text.Json.Nodes; +using System.Threading; +using System.Threading.Tasks; +using ManagedCommon; +using Microsoft.CmdPal.Common.Services; +using Microsoft.CmdPal.UI.ViewModels.Services; +using Microsoft.CmdPal.UI.ViewModels.Services.JsonRpc; +using Microsoft.CommandPalette.Extensions; +using Windows.ApplicationModel; + +namespace Microsoft.CmdPal.UI.ViewModels.Models; + +/// +/// Manages a single JavaScript/TypeScript extension running as an isolated Node.js +/// process and presents it to the CmdPal host as an . +/// The process is spawned with stdio redirection and driven over a +/// ; the forwards +/// provider calls to the extension. +/// +public sealed partial class JSExtensionWrapper : IExtensionWrapper, IDisposable +{ + // Consecutive crashes above this threshold mark the extension unhealthy. + private const int MaxConsecutiveCrashes = 3; + + // Default Node.js inspector port. Auto-assigned ports start at 9229 (the first + // Interlocked.Increment below yields 9229 from this seed). + private static int _nextDebugPort = 9228; + + private readonly JSExtensionManifest _manifest; + private readonly string _manifestDirectory; + private readonly Lock _lock = new(); + private readonly List _providerTypes = []; + + private Process? _nodeProcess; + private JsonRpcConnection? _connection; + private JSCommandProviderProxy? _commandProviderProxy; + private Task? _startInProgress; + private bool _isDisposed; + private bool _stopping; + private int _consecutiveCrashCount; + + /// + /// Initializes a new instance of the class. + /// + /// The parsed and validated extension manifest. + /// The directory that contains the extension's package.json. + public JSExtensionWrapper(JSExtensionManifest manifest, string manifestDirectory) + { + _manifest = manifest ?? throw new ArgumentNullException(nameof(manifest)); + _manifestDirectory = manifestDirectory ?? throw new ArgumentNullException(nameof(manifestDirectory)); + + // JS extensions currently expose a single command provider. + AddProviderType(ProviderType.Commands); + } + + /// + /// Raised when the underlying Node.js process exits unexpectedly (a crash), after the + /// wrapper has torn down its process and connection handles. It is not raised for an + /// intentional stop via . The service uses this to remove the + /// now-dead provider and decide whether to restart or disable the extension. + /// + public event EventHandler? ProcessExited; + + public string PackageDisplayName => _manifest.EffectiveDisplayName; + + public string ExtensionDisplayName => _manifest.EffectiveDisplayName; + + public string PackageFullName => $"js!{_manifest.Name}"; + + public string PackageFamilyName => $"js!{_manifest.Name}"; + + public string Publisher => _manifest.Publisher ?? "Unknown"; + + public string ExtensionClassId + { + get + { + // Derive a stable identifier from the manifest name. + if (string.IsNullOrWhiteSpace(_manifest.Name)) + { + return "unknown"; + } + + var hash = SHA256.HashData(Encoding.UTF8.GetBytes(_manifest.Name)); + return $"js-{Convert.ToHexString(hash)[..32]}"; + } + } + + public DateTimeOffset InstalledDate + { + get + { + try + { + var manifestPath = Path.Combine(_manifestDirectory, "package.json"); + if (File.Exists(manifestPath)) + { + return File.GetCreationTimeUtc(manifestPath); + } + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + // Fall through to the default below. + } + + return DateTimeOffset.UtcNow; + } + } + + public PackageVersion Version + { + get + { + if (string.IsNullOrWhiteSpace(_manifest.Version)) + { + return new PackageVersion { Major = 1, Minor = 0, Build = 0, Revision = 0 }; + } + + var parts = _manifest.Version.Split('.'); + return new PackageVersion + { + Major = parts.Length > 0 && ushort.TryParse(parts[0], out var major) ? major : (ushort)1, + Minor = parts.Length > 1 && ushort.TryParse(parts[1], out var minor) ? minor : (ushort)0, + Build = parts.Length > 2 && ushort.TryParse(parts[2], out var build) ? build : (ushort)0, + Revision = 0, + }; + } + } + + public string ExtensionUniqueId => $"js!{_manifest.Name}"; + + /// + /// Gets the directory that contains the extension's package.json. + /// + internal string ManifestDirectory => _manifestDirectory; + + /// + /// Gets the manifest this extension was loaded with. Used by the service to detect a + /// manifest edit during an explicit refresh and reload the extension when it changed. + /// + internal JSExtensionManifest Manifest => _manifest; + + /// + /// Gets the normalized identity key for this extension, used to enforce cross-extension + /// uniqueness during discovery. + /// + internal string NameKey => _manifest.NameKey; + + /// + /// Gets the number of times this extension has recorded a consecutive crash + /// without a successful start in between. + /// + internal int ConsecutiveCrashCount + { + get + { + lock (_lock) + { + return _consecutiveCrashCount; + } + } + } + + /// + /// Gets a value indicating whether the extension is considered healthy. It + /// becomes unhealthy after more than + /// consecutive crashes and stays that way until a successful start resets the counter. + /// + internal bool IsHealthy { get; private set; } = true; + + /// + /// Gets the capabilities advertised by the extension in its initialize response. + /// Currently advisory: recorded for diagnostics but not used to gate behavior. + /// + internal IReadOnlyList Capabilities { get; private set; } = []; + + public bool IsRunning() + { + lock (_lock) + { + return IsRunningLocked(); + } + } + + private bool IsRunningLocked() + { + if (_nodeProcess is null || _connection is null) + { + return false; + } + + try + { + return !_nodeProcess.HasExited; + } + catch (InvalidOperationException) + { + return false; + } + } + + public Task StartExtensionAsync() + { + lock (_lock) + { + ObjectDisposedException.ThrowIf(_isDisposed, this); + + if (IsRunningLocked()) + { + return Task.CompletedTask; + } + + // Single-flight: a concurrent caller (for example GetProviderAsync calling in + // right after the service starts the wrapper) joins the in-progress start + // instead of spawning a second Node process. The start body runs on the thread + // pool so no process is spawned while this lock is held; the task is cleared + // when it completes so a later restart can start again. + _startInProgress ??= Task.Run(RunStartAsync); + return _startInProgress; + } + } + + private async Task RunStartAsync() + { + try + { + await StartCoreAsync().ConfigureAwait(false); + } + finally + { + lock (_lock) + { + _startInProgress = null; + } + } + } + + private async Task StartCoreAsync() + { + lock (_lock) + { + // The wrapper may have been disposed, or another start may have completed, + // between scheduling this start and running it. + if (_isDisposed || IsRunningLocked()) + { + return; + } + } + + Logger.LogDebug($"Starting JS extension {_manifest.EffectiveDisplayName}"); + + var entryPoint = _manifest.EntryPointPath ?? Path.Combine(_manifestDirectory, _manifest.Main ?? string.Empty); + if (!File.Exists(entryPoint)) + { + Logger.LogError($"Entry point not found for {_manifest.Name}: {entryPoint}"); + return; + } + + // Launch through the Phase 1 SDK bootstrap when it is installed so the bootstrap + // claims stdout for the protocol before the extension entry is dynamically imported. + // The effective launch command is: + // node [--inspect=] "" "" + // and, when the bootstrap cannot be resolved: + // node [--inspect=] "" + var bootstrapScript = ResolveBootstrapScript(_manifestDirectory); + if (bootstrapScript is null) + { + Logger.LogWarning( + $"Bootstrap loader not found for {_manifest.Name}; launching the entry directly. A stray top-level stdout write can corrupt the protocol until the SDK bootstrap is installed."); + } + + // Resolve an absolute node.exe from PATH rather than launching the bare name + // "node". The process working directory is the extension's own folder, so a bare + // name could otherwise resolve a node.exe planted there; an absolute path avoids + // that and lets us report a specific "Node.js not found" error. + var nodeExecutable = NodeRuntimeLocator.ResolveNodeExecutable(); + if (nodeExecutable is null) + { + Logger.LogError( + $"Node.js runtime (node.exe) was not found on PATH; cannot start JS extension {_manifest.Name}. Install Node.js and ensure it is on PATH."); + return; + } + + Process? nodeProcess = null; + JsonRpcConnection? connection = null; + try + { + var psi = new ProcessStartInfo + { + FileName = nodeExecutable, + Arguments = BuildNodeArguments(entryPoint, bootstrapScript), + UseShellExecute = false, + RedirectStandardInput = true, + RedirectStandardOutput = true, + RedirectStandardError = true, + CreateNoWindow = true, + WorkingDirectory = _manifestDirectory, + }; + + nodeProcess = Process.Start(psi); + if (nodeProcess is null) + { + Logger.LogError($"Failed to start Node.js process for {_manifest.Name}"); + return; + } + + connection = new JsonRpcConnection( + nodeProcess.StandardOutput.BaseStream, + nodeProcess.StandardInput.BaseStream, + nodeProcess.StandardError.BaseStream); + + connection.Error += OnConnectionError; + connection.Disconnected += OnConnectionDisconnected; + + var disposedDuringStart = false; + lock (_lock) + { + // If a dispose landed while the process was starting, do not resurrect the + // wrapper by assigning the fresh handles. Reap the just-started process + // below instead so it cannot leak past disposal. + if (_isDisposed || _stopping) + { + disposedDuringStart = true; + } + else + { + _nodeProcess = nodeProcess; + _connection = connection; + + // Create the provider proxy before the initialize handshake so its host + // notification handlers are registered in time to receive notifications + // (logs, statuses, clipboard requests, items-changed) that the extension + // emits while it activates during initialize. + _commandProviderProxy = new JSCommandProviderProxy( + connection, + _manifest.Name ?? "unknown", + _manifest.EffectiveDisplayName, + _manifest.Icon); + } + } + + if (disposedDuringStart) + { + Logger.LogDebug($"JS extension {_manifest.Name} was disposed while starting; reaping the new process."); + ReapOrphanedStart(nodeProcess, connection); + return; + } + + connection.StartListening(); + + var initResponse = await connection.SendRequestAsync( + "initialize", + new JsonObject { ["extensionId"] = _manifest.Name }, + CancellationToken.None).ConfigureAwait(false); + + if (initResponse.Error is not null) + { + Logger.LogError($"Initialization failed for {_manifest.Name}: {initResponse.Error.Message}"); + SignalDispose(); + return; + } + + RecordAdvertisedCapabilities(initResponse.Result); + + // Thread the real provider metadata from the handshake into the proxy so the + // author-specified frozen value flows through instead of the wire default. + var providerMetadata = ExtractProviderMetadata(initResponse.Result); + if (providerMetadata is { } metadata) + { + JSCommandProviderProxy? proxy; + lock (_lock) + { + proxy = _commandProviderProxy; + } + + proxy?.SetProviderMetadata(metadata); + } + + // A successful start clears the consecutive-crash history. + ResetCrashCount(); + + Logger.LogInfo($"Successfully started JS extension {_manifest.EffectiveDisplayName}"); + } + catch (Exception ex) + { + Logger.LogError($"Failed to start JS extension {_manifest.Name}: {ex.Message}"); + + try + { + if (nodeProcess is not null && !nodeProcess.HasExited) + { + nodeProcess.Kill(entireProcessTree: true); + } + } + catch (Exception killEx) when (killEx is InvalidOperationException or System.ComponentModel.Win32Exception) + { + // Best effort. + } + + SignalDispose(); + } + } + + private void ReapOrphanedStart(Process nodeProcess, JsonRpcConnection connection) + { + connection.Error -= OnConnectionError; + connection.Disconnected -= OnConnectionDisconnected; + + try + { + connection.Dispose(); + } + catch (Exception ex) + { + Logger.LogDebug($"Error disposing orphaned connection for {_manifest.Name}: {ex.Message}"); + } + + try + { + if (!nodeProcess.HasExited) + { + nodeProcess.Kill(entireProcessTree: true); + nodeProcess.WaitForExit(2000); + } + } + catch (Exception ex) when (ex is InvalidOperationException or System.ComponentModel.Win32Exception) + { + // Best effort. + } + + nodeProcess.Dispose(); + } + + public void SignalDispose() + { + Process? process; + JsonRpcConnection? connection; + JSCommandProviderProxy? proxy; + + lock (_lock) + { + _isDisposed = true; + _stopping = true; + process = _nodeProcess; + connection = _connection; + proxy = _commandProviderProxy; + _nodeProcess = null; + _connection = null; + _commandProviderProxy = null; + } + + TearDown(process, connection, proxy); + } + + public void Dispose() => SignalDispose(); + + public IExtension? GetExtensionObject() + { + // JS extensions have no WinRT COM object; the wrapper itself is the bridge. + return null; + } + + public void AddProviderType(ProviderType providerType) + { + lock (_lock) + { + if (!_providerTypes.Contains(providerType)) + { + _providerTypes.Add(providerType); + } + } + } + + public bool HasProviderType(ProviderType providerType) + { + lock (_lock) + { + return _providerTypes.Contains(providerType); + } + } + + public async Task GetProviderAsync() + where T : class + { + if (typeof(T) != typeof(ICommandProvider)) + { + return null; + } + + await StartExtensionAsync().ConfigureAwait(false); + + lock (_lock) + { + if (_connection is null || !IsRunningLocked()) + { + return null; + } + + _commandProviderProxy ??= new JSCommandProviderProxy( + _connection, + _manifest.Name ?? "unknown", + _manifest.EffectiveDisplayName, + _manifest.Icon); + return _commandProviderProxy as T; + } + } + + public async Task> GetListOfProvidersAsync() + where T : class + { + var provider = await GetProviderAsync().ConfigureAwait(false); + return provider is not null ? [provider] : []; + } + + /// + /// Records a consecutive crash and updates . Extracted so the + /// crash-counter state machine can be exercised without spawning a Node.js process. + /// + /// The new consecutive crash count. + internal int RecordUnexpectedExit() + { + lock (_lock) + { + _consecutiveCrashCount++; + if (_consecutiveCrashCount > MaxConsecutiveCrashes) + { + IsHealthy = false; + } + + return _consecutiveCrashCount; + } + } + + /// + /// Resets the consecutive crash counter and marks the extension healthy again. + /// + internal void ResetCrashCount() + { + lock (_lock) + { + _consecutiveCrashCount = 0; + IsHealthy = true; + } + } + + private void OnConnectionError(object? sender, JsonRpcErrorEventArgs e) + { + Logger.LogError($"JSON-RPC error in {_manifest.Name}: {e.Exception.Message}"); + } + + private void OnConnectionDisconnected(object? sender, EventArgs e) + { + Process? process; + JsonRpcConnection? connection; + JSCommandProviderProxy? proxy; + + lock (_lock) + { + // Ignore disconnections that we triggered while stopping or disposing. + if (_stopping || _isDisposed) + { + return; + } + + _consecutiveCrashCount++; + Logger.LogWarning($"Node.js process for {_manifest.Name} disconnected unexpectedly (crash #{_consecutiveCrashCount})"); + + if (_consecutiveCrashCount > MaxConsecutiveCrashes) + { + IsHealthy = false; + Logger.LogError($"JS extension {_manifest.Name} marked unhealthy after {_consecutiveCrashCount} consecutive crashes"); + } + + process = _nodeProcess; + connection = _connection; + proxy = _commandProviderProxy; + _nodeProcess = null; + _connection = null; + _commandProviderProxy = null; + } + + // This runs on the connection's read-loop thread, and JsonRpcConnection.Dispose() + // joins that thread. Tear the handles down and notify the service on a background + // thread to avoid a self-join and to keep the read loop from blocking on itself. + _ = Task.Run(() => + { + TearDown(process, connection, proxy); + ProcessExited?.Invoke(this, EventArgs.Empty); + }); + } + + private void TearDown(Process? process, JsonRpcConnection? connection, JSCommandProviderProxy? proxy) + { + // Dispose the provider proxy first so it detaches its notification handlers and + // hides any active statuses while the host is still valid, including when the + // extension process exits unexpectedly. The proxy is idempotent, so this does not + // double-dispose if the service also disposes it during provider teardown. + if (proxy is not null) + { + try + { + proxy.Dispose(); + } + catch (Exception ex) + { + Logger.LogDebug($"Error disposing provider proxy for {_manifest.Name}: {ex.Message}"); + } + } + + if (connection is not null) + { + connection.Error -= OnConnectionError; + connection.Disconnected -= OnConnectionDisconnected; + + try + { + var stillRunning = process is not null && !process.HasExited; + if (stillRunning) + { + // Ask the extension to clean up, giving it a short grace period. + connection.SendNotificationAsync("dispose", null, CancellationToken.None) + .Wait(TimeSpan.FromSeconds(2)); + } + } + catch (Exception ex) when (ex is AggregateException or InvalidOperationException or JsonRpcException) + { + Logger.LogWarning($"Error sending dispose notification to {_manifest.Name}: {ex.Message}"); + } + + try + { + connection.Dispose(); + } + catch (Exception ex) + { + Logger.LogDebug($"Error disposing JSON-RPC connection for {_manifest.Name}: {ex.Message}"); + } + } + + if (process is not null) + { + try + { + if (!process.HasExited) + { + process.Kill(entireProcessTree: true); + process.WaitForExit(2000); + } + } + catch (Exception ex) when (ex is InvalidOperationException or System.ComponentModel.Win32Exception) + { + Logger.LogWarning($"Error terminating Node.js process for {_manifest.Name}: {ex.Message}"); + } + + process.Dispose(); + } + } + + private static JsonElement? ExtractProviderMetadata(JsonElement? result) + { + if (result is not { } initResult || + initResult.ValueKind != JsonValueKind.Object) + { + return null; + } + + if ((initResult.TryGetProperty("provider", out var provider) || + initResult.TryGetProperty("Provider", out provider)) && + provider.ValueKind == JsonValueKind.Object) + { + // Clone so the metadata survives the disposal of the response document. + return provider.Clone(); + } + + return null; + } + + private void RecordAdvertisedCapabilities(JsonElement? result) + { + if (result is not { } initResult || + initResult.ValueKind != JsonValueKind.Object || + !initResult.TryGetProperty("capabilities", out var capsElement) || + capsElement.ValueKind != JsonValueKind.Array) + { + return; + } + + var capabilities = new List(); + foreach (var cap in capsElement.EnumerateArray()) + { + if (cap.ValueKind == JsonValueKind.String) + { + var value = cap.GetString(); + if (!string.IsNullOrEmpty(value)) + { + capabilities.Add(value); + } + } + } + + Capabilities = capabilities; + if (capabilities.Count > 0) + { + Logger.LogInfo($"Extension {_manifest.Name} advertised capabilities: {string.Join(", ", capabilities)}"); + } + } + + private string BuildNodeArguments(string entryPoint, string? bootstrapScript) + { + // node [--inspect=] "" "" when the bootstrap resolves, + // otherwise node [--inspect=] "". The bootstrap reads the entry from + // process.argv[2]; Node runtime flags such as --inspect never enter process.argv, + // so the entry stays at argv[2] regardless of debug mode. + var target = bootstrapScript is null + ? $"\"{entryPoint}\"" + : $"\"{bootstrapScript}\" \"{entryPoint}\""; + + if (_manifest.Debug) + { + var port = _manifest.DebugPort ?? Interlocked.Increment(ref _nextDebugPort); + Logger.LogInfo($"Debug mode enabled for {_manifest.Name} on inspector port {port}"); + return $"--inspect={port} {target}"; + } + + return target; + } + + /// + /// Resolves the Phase 1 SDK bootstrap loader for an installed extension. The bootstrap + /// claims and guards stdout before it dynamically imports the extension entry, so a + /// static top-level stdout write cannot corrupt the JSON-RPC framing. Resolution is + /// relative to the extension's installed SDK + /// (<manifestDirectory>/node_modules/@microsoft/cmdpal-sdk), preferring the + /// package's declared bin entry and falling back to the known published + /// artifacts. Returns when the SDK or its bootstrap is not present. + /// + internal static string? ResolveBootstrapScript(string manifestDirectory) + { + if (string.IsNullOrEmpty(manifestDirectory)) + { + return null; + } + + var sdkRoot = Path.Combine(manifestDirectory, "node_modules", "@microsoft", "cmdpal-sdk"); + if (!Directory.Exists(sdkRoot)) + { + return null; + } + + // Prefer the SDK package's declared bin entry so the launch tracks the published + // contract rather than a hardcoded artifact path. + var fromBin = ResolveBootstrapFromPackageJson(sdkRoot); + if (fromBin is not null && File.Exists(fromBin)) + { + return fromBin; + } + + foreach (var candidate in new[] + { + Path.Combine(sdkRoot, "dist", "runtime", "bootstrap.js"), + Path.Combine(sdkRoot, "bin", "cmdpal-bootstrap.mjs"), + }) + { + if (File.Exists(candidate)) + { + return candidate; + } + } + + return null; + } + + private static string? ResolveBootstrapFromPackageJson(string sdkRoot) + { + var packageJsonPath = Path.Combine(sdkRoot, "package.json"); + if (!File.Exists(packageJsonPath)) + { + return null; + } + + try + { + using var document = JsonDocument.Parse(File.ReadAllText(packageJsonPath)); + if (!document.RootElement.TryGetProperty("bin", out var bin)) + { + return null; + } + + string? relative = null; + if (bin.ValueKind == JsonValueKind.String) + { + relative = bin.GetString(); + } + else if (bin.ValueKind == JsonValueKind.Object) + { + if (bin.TryGetProperty("cmdpal-bootstrap", out var named) && named.ValueKind == JsonValueKind.String) + { + relative = named.GetString(); + } + else + { + foreach (var property in bin.EnumerateObject()) + { + if (property.Value.ValueKind == JsonValueKind.String) + { + relative = property.Value.GetString(); + break; + } + } + } + } + + if (string.IsNullOrEmpty(relative)) + { + return null; + } + + return Path.GetFullPath(Path.Combine(sdkRoot, relative)); + } + catch (Exception ex) when (ex is IOException or JsonException or UnauthorizedAccessException) + { + return null; + } + } +} diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Services/DirectoryLifecycleGate.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Services/DirectoryLifecycleGate.cs new file mode 100644 index 0000000000..e8c096d753 --- /dev/null +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Services/DirectoryLifecycleGate.cs @@ -0,0 +1,221 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Collections.Generic; +using System.IO; +using System.Threading; +using System.Threading.Tasks; + +namespace Microsoft.CmdPal.UI.ViewModels.Services; + +/// +/// Serializes all lifecycle operations (initial load, refresh, crash-restart, +/// hot-reload, and removal) for a single extension directory so that concurrent +/// triggers cannot launch duplicate processes for the same extension. The gate is +/// keyed by the canonical directory path (case-insensitive), matching the rest of +/// the service's directory comparisons. +/// +/// +/// Entries are reference counted. An entry stays alive while any caller holds or is +/// waiting on it, so during a concurrent acquire never disposes +/// a semaphore out from under a waiter (which would surface as an +/// ). The backing semaphore is disposed only +/// once the last reference is released after a removal, or when the gate itself is +/// disposed. +/// +internal sealed partial class DirectoryLifecycleGate : IDisposable +{ + private readonly Lock _lock = new(); + private readonly Dictionary _entries = new(StringComparer.OrdinalIgnoreCase); + private bool _disposed; + + /// + /// Produces the canonical key for a directory: an absolute path with any trailing + /// separator trimmed. Invalid paths fall back to the trimmed original so callers + /// still get a stable key rather than an exception. + /// + /// The directory to canonicalize. + /// The canonical key used to group lifecycle operations. + public static string Canonicalize(string directory) + { + if (string.IsNullOrEmpty(directory)) + { + return string.Empty; + } + + try + { + return Path.TrimEndingDirectorySeparator(Path.GetFullPath(directory)); + } + catch (Exception ex) when (ex is ArgumentException or NotSupportedException or PathTooLongException) + { + return Path.TrimEndingDirectorySeparator(directory); + } + } + + /// + /// Acquires exclusive access to the lifecycle of a directory. Dispose the returned + /// handle to release it. Operations for different directories run concurrently; + /// operations for the same directory are serialized. + /// + /// The extension directory whose lifecycle is being changed. + /// A token that cancels the wait. + /// A handle that releases the gate when disposed. + public async Task AcquireAsync(string directory, CancellationToken cancellationToken) + { + var key = Canonicalize(directory); + Entry entry; + lock (_lock) + { + ObjectDisposedException.ThrowIf(_disposed, this); + + if (!_entries.TryGetValue(key, out var existing)) + { + existing = new Entry(); + _entries[key] = existing; + } + + existing.Refs++; + entry = existing; + } + + try + { + await entry.Semaphore.WaitAsync(cancellationToken).ConfigureAwait(false); + } + catch + { + // The wait was canceled (or failed); drop the reference we took so the + // entry can be cleaned up and does not leak. + ReleaseReference(key, entry, releaseSemaphore: false); + throw; + } + + return new Releaser(this, key, entry); + } + + /// + /// Marks a directory's gate entry for removal. When no one holds or awaits it the + /// entry is evicted and its semaphore disposed immediately. When a holder or waiter + /// still exists the entry is left in place (marked removed) so that any operation + /// already queued behind it, and any operation that arrives before it drains, keep + /// serializing on the same semaphore. The entry is evicted only once its last + /// reference is released (see ). This guarantees a new + /// generation for the directory strictly supersedes the prior one and can never run + /// concurrently with it. + /// + /// The directory whose gate entry should be released. + public void Remove(string directory) + { + var key = Canonicalize(directory); + lock (_lock) + { + if (_entries.TryGetValue(key, out var entry)) + { + entry.Removed = true; + if (entry.Refs == 0) + { + _entries.Remove(key); + entry.Semaphore.Dispose(); + } + + // Otherwise keep the entry in the map so later acquires reuse it and stay + // serialized behind the drain; ReleaseReference evicts it at Refs == 0. + } + } + } + + public void Dispose() + { + lock (_lock) + { + if (_disposed) + { + return; + } + + _disposed = true; + foreach (var entry in _entries.Values) + { + entry.Removed = true; + if (entry.Refs == 0) + { + entry.Semaphore.Dispose(); + } + } + + _entries.Clear(); + } + } + + private void ReleaseReference(string key, Entry entry, bool releaseSemaphore) + { + if (releaseSemaphore) + { + try + { + entry.Semaphore.Release(); + } + catch (ObjectDisposedException) + { + // The gate was disposed while this operation held it; nothing to release. + } + } + + lock (_lock) + { + entry.Refs--; + if (entry.Refs == 0 && entry.Removed) + { + // The last reference to a removed entry is gone; evict it so the next + // acquire for this directory starts a fresh generation, and dispose the + // semaphore. Guard against evicting a different entry that may have taken + // this key (belt and suspenders; a removed entry is never replaced while + // it is still present). + if (_entries.TryGetValue(key, out var current) && ReferenceEquals(current, entry)) + { + _entries.Remove(key); + } + + entry.Semaphore.Dispose(); + } + } + } + + private sealed class Entry + { + public SemaphoreSlim Semaphore { get; } = new(1, 1); + + public int Refs { get; set; } + + public bool Removed { get; set; } + } + + private sealed partial class Releaser : IDisposable + { + private readonly DirectoryLifecycleGate _gate; + private readonly string _key; + private readonly Entry _entry; + private bool _released; + + public Releaser(DirectoryLifecycleGate gate, string key, Entry entry) + { + _gate = gate; + _key = key; + _entry = entry; + } + + public void Dispose() + { + if (_released) + { + return; + } + + _released = true; + _gate.ReleaseReference(_key, _entry, releaseSemaphore: true); + } + } +} diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Services/HotReloadDebouncer.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Services/HotReloadDebouncer.cs new file mode 100644 index 0000000000..be83cff4cb --- /dev/null +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Services/HotReloadDebouncer.cs @@ -0,0 +1,180 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Collections.Generic; +using System.Threading; + +namespace Microsoft.CmdPal.UI.ViewModels.Services; + +/// +/// Coalesces rapid file-change notifications per key (extension directory) into a +/// single delayed callback. Changes under node_modules are ignored. Used by +/// to debounce hot-reloads while a developer saves. +/// +internal sealed partial class HotReloadDebouncer : IDisposable +{ + private readonly TimeSpan _delay; + private readonly Action _callback; + private readonly Lock _lock = new(); + private readonly Dictionary _timers = new(StringComparer.OrdinalIgnoreCase); + private bool _disposed; + + // Monotonic load generation. A timer captures the generation current when it is + // (re)armed; when the service stops it advances the generation so a callback that was + // already queued before the stop is dropped instead of firing a hot-reload against the + // next load generation. + private long _generation; + + /// + /// Initializes a new instance of the class. + /// + /// Invoked with the key once a key has been quiet for the debounce delay. + /// The debounce window. Defaults to 500 ms when null. + public HotReloadDebouncer(Action callback, TimeSpan? delay = null) + { + _callback = callback ?? throw new ArgumentNullException(nameof(callback)); + _delay = delay ?? TimeSpan.FromMilliseconds(500); + } + + /// + /// Returns a value indicating whether the given path represents a change that should + /// trigger a hot-reload (that is, it is not inside a node_modules directory). + /// + /// The full path of the changed file. + /// True when the change is relevant; otherwise false. + public static bool IsRelevantChange(string changedPath) + { + if (string.IsNullOrEmpty(changedPath)) + { + return false; + } + + return changedPath.IndexOf("node_modules", StringComparison.OrdinalIgnoreCase) < 0; + } + + /// + /// Notifies the debouncer of a change to for the given key. + /// Irrelevant changes are dropped; relevant ones (re)start the debounce window. + /// + /// The key that groups the change, typically the extension directory. + /// The full path of the changed file. + public void Notify(string key, string changedPath) + { + if (string.IsNullOrEmpty(key) || !IsRelevantChange(changedPath)) + { + return; + } + + lock (_lock) + { + if (_disposed) + { + return; + } + + if (_timers.TryGetValue(key, out var existing)) + { + existing.Change(_delay, Timeout.InfiniteTimeSpan); + return; + } + + // Capture the current generation so a callback that fires after a stop, but + // whose timer was armed before it, is dropped. A stop clears the timer map, so + // any timer found in the map above was armed in the current generation. + var state = new PendingReload(key, _generation); + _timers[key] = new Timer(OnTimerElapsed, state, _delay, Timeout.InfiniteTimeSpan); + } + } + + /// + /// Cancels any pending debounce for the given key. + /// + /// The key to cancel. + public void Cancel(string key) + { + lock (_lock) + { + if (_timers.TryGetValue(key, out var timer)) + { + timer.Dispose(); + _timers.Remove(key); + } + } + } + + /// + /// Cancels every pending debounce and advances the load generation. Callbacks whose + /// timer was armed before this call are dropped even if they had already been queued to + /// the thread pool, so a hot-reload notified in a prior generation cannot fire against + /// the next one. Used when the service stops between load cycles. + /// + public void CancelAll() + { + lock (_lock) + { + if (_disposed) + { + return; + } + + _generation++; + foreach (var timer in _timers.Values) + { + timer.Dispose(); + } + + _timers.Clear(); + } + } + + public void Dispose() + { + lock (_lock) + { + if (_disposed) + { + return; + } + + _disposed = true; + foreach (var timer in _timers.Values) + { + timer.Dispose(); + } + + _timers.Clear(); + } + } + + private void OnTimerElapsed(object? state) + { + var pending = (PendingReload)state!; + + lock (_lock) + { + if (_disposed) + { + return; + } + + // Drop a callback captured in an earlier generation (the service stopped after + // this timer was armed), so it cannot drive a reload in the current generation. + if (pending.Generation != _generation) + { + return; + } + + if (_timers.TryGetValue(pending.Key, out var timer)) + { + timer.Dispose(); + _timers.Remove(pending.Key); + } + } + + _callback(pending.Key); + } + + private readonly record struct PendingReload(string Key, long Generation); +} diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Services/JsonRpcExtensionService.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Services/JsonRpcExtensionService.cs new file mode 100644 index 0000000000..6e8e4d5087 --- /dev/null +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Services/JsonRpcExtensionService.cs @@ -0,0 +1,1798 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.IO; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using ManagedCommon; +using Microsoft.CmdPal.Common.Services; +using Microsoft.CmdPal.UI.ViewModels.Models; +using Microsoft.CommandPalette.Extensions; +using Windows.Foundation; + +namespace Microsoft.CmdPal.UI.ViewModels.Services; + +/// +/// Extension service that manages JavaScript/TypeScript extensions. Each extension +/// runs as its own Node.js process communicating over JSON-RPC 2.0 via stdio. +/// The service discovers extensions in a well-known directory, watches that directory +/// for install/uninstall, and hot-reloads an extension (debounced) when its source +/// files change. +/// +/// +/// All lifecycle transitions for a single extension directory (initial load, refresh, +/// crash-restart, hot-reload, and removal) are serialized through a per-directory +/// so concurrent triggers can never launch +/// duplicate processes for the same extension. The synchronous +/// only guards in-memory collection mutations and is never held across an await or a +/// process launch. +/// +public sealed partial class JsonRpcExtensionService : IExtensionService, IDisposable +{ + // Consecutive crashes above this threshold disable an extension instead of restarting it. + private const int MaxRestartAttempts = 3; + + // Source-file extensions that trigger a hot-reload, per the manifest contract. + private static readonly string[] WatchedSourceExtensions = [".js", ".mjs", ".cjs"]; + + // Path segments that never carry a relevant manifest or source change. Churn under + // these (npm writing hundreds of files under node_modules during an install, or git + // metadata) must not drive discovery or hot-reload, or it causes a restart storm. + private static readonly string[] IgnoredDirectorySegments = ["node_modules", ".git"]; + + // How many times a newly appeared package is re-checked for a parseable manifest + // before giving up, and how long to wait between checks. This lets a slow install + // (directory created first, manifest written later) settle before it is loaded. + private const int ManifestStabilityAttempts = 20; + private static readonly TimeSpan ManifestStabilityDelay = TimeSpan.FromMilliseconds(250); + + private static readonly string ExtensionsPath = GetDefaultExtensionsPath(); + + private readonly TaskScheduler _taskScheduler; + private readonly Lock _extensionsLock = new(); + private readonly List _extensions = []; + private readonly List _providerWrappers = []; + private readonly HashSet _disabledExtensions = new(StringComparer.Ordinal); + + // Provider-id (normalized manifest name key) reservations shared by every + // registration path. Consulted and claimed atomically under _extensionsLock so a + // duplicate id can never register regardless of how it arrives (initial scan, + // refresh, dynamic install, hot-reload, or crash-restart). + private readonly ProviderIdReservations _providerIds = new(); + + // Consecutive crash-restart attempts per canonical extension directory. Reset when + // an extension is (re)loaded through a non-crash path (initial discovery, install, + // or source hot-reload). + private readonly Dictionary _crashCounts = new(StringComparer.OrdinalIgnoreCase); + + private readonly Lock _sourceWatcherLock = new(); + private readonly Dictionary _sourceFileWatchers = new(StringComparer.OrdinalIgnoreCase); + private readonly HotReloadDebouncer _hotReloadDebouncer; + + // Reusable cancellation for the current load cycle. A single CancellationTokenSource + // can only be canceled once, so stop-then-load-again would otherwise hand out a + // permanently canceled token; this wrapper swaps in a fresh source per cycle. + private readonly ReloadCancellation _reload = new(); + + private readonly DirectoryLifecycleGate _directoryGate = new(); + + // Single ordered dispatch path for OnProviderAdded/OnProviderRemoved so consumers can + // never observe a provider addition before a removal that was raised ahead of it, even + // when the two originate on different threads. + private readonly SerialNotificationDispatcher _notifications = new(); + + private FileSystemWatcher? _directoryWatcher; + private bool _disposed; + + // Set true, under _extensionsLock, once shutdown has cleared the collections. A start + // that completes after this point must not register its extension (which would leak a + // Node process and watcher past shutdown); it tears the fresh instance down instead. + private bool _shuttingDown; + + public JsonRpcExtensionService(TaskScheduler taskScheduler) + { + _taskScheduler = taskScheduler; + _hotReloadDebouncer = new HotReloadDebouncer(directory => _ = HotReloadExtensionAsync(directory)); + } + + public event TypedEventHandler>? OnProviderAdded; + + public event TypedEventHandler>? OnProviderRemoved; + + /// + /// The action to take after an extension's Node.js process has crashed. + /// + internal enum CrashAction + { + /// Restart the extension with a fresh process and connection. + Restart, + + /// Stop restarting the extension and leave it disabled. + Disable, + } + + /// + /// The result of attempting to register a freshly started extension into the service's + /// in-memory collections under the extensions lock. + /// + private enum RegistrationOutcome + { + /// The extension was added and its provider id reserved. + Added, + + /// Another extension is already loaded from the same directory. + DuplicateDirectory, + + /// Another directory already owns this extension's provider id. + DuplicateId, + + /// Shutdown began before the extension could be registered. + Stopping, + } + + public async Task> LoadProvidersAsync(CancellationToken ct) + { + if (ct.IsCancellationRequested) + { + return []; + } + + // Begin a fresh load cycle. This replaces a token that a previous stop left + // canceled, so a load after a stop actually runs. + _reload.BeginCycle(); + + // A new load cycle clears the shutting-down guard so registrations are accepted + // again after a previous SignalStopAsync. + lock (_extensionsLock) + { + _shuttingDown = false; + } + + var sw = Stopwatch.StartNew(); + + if (!EnsureExtensionsDirectory()) + { + return []; + } + + // Start the watcher before scanning so a package installed while the scan runs + // is still observed (the per-directory gate and the already-loaded check make a + // watcher-driven load and a scan-driven load for the same directory idempotent). + StartDirectoryWatcher(); + + var wrappers = new List(); + foreach (var (directory, manifest) in DiscoverAcceptedManifests(ExtensionsPath)) + { + if (ct.IsCancellationRequested || _reload.IsStopRequested) + { + break; + } + + var wrapper = await AddExtensionGatedAsync(directory, manifest, ct).ConfigureAwait(false); + if (wrapper is not null) + { + wrappers.Add(wrapper); + } + } + + // Reconcile once more to pick up anything installed during the scan/watch gap. + var stragglers = await AddDiscoveredNotLoadedAsync(ct).ConfigureAwait(false); + wrappers.AddRange(stragglers); + + sw.Stop(); + Logger.LogInfo($"JsonRpcExtensionService: Loaded {wrappers.Count} extension(s) in {sw.ElapsedMilliseconds} ms"); + + return wrappers; + } + + public Task SignalStopAsync() + { + // Request cancellation first so any in-flight, delayed watcher handlers bail out + // before they start an extension after we have already begun shutting down. + _reload.Stop(); + + StopDirectoryWatcher(); + StopAllSourceFileWatchers(); + + List toStop; + lock (_extensionsLock) + { + _shuttingDown = true; + toStop = [.. _extensions]; + _extensions.Clear(); + _providerWrappers.Clear(); + _crashCounts.Clear(); + _providerIds.Clear(); + } + + foreach (var ext in toStop) + { + try + { + ext.ProcessExited -= OnExtensionProcessExited; + ext.SignalDispose(); + } + catch (Exception ex) + { + Logger.LogError($"Failed to stop JS extension {ext.ExtensionDisplayName}: {ex.Message}"); + } + } + + return Task.CompletedTask; + } + + public Task> GetInstalledExtensionsAsync(bool includeDisabledExtensions = false) + { + lock (_extensionsLock) + { + var result = includeDisabledExtensions + ? _extensions.Cast().ToList() + : _extensions.Where(e => !_disabledExtensions.Contains(e.ExtensionUniqueId)).Cast().ToList(); + + return Task.FromResult>(result); + } + } + + public async Task> RefreshInstalledExtensionsAsync(bool includeDisabledExtensions = false) + { + if (EnsureExtensionsDirectory()) + { + // Add newly installed extensions. + var added = await AddDiscoveredNotLoadedAsync(CancellationToken.None).ConfigureAwait(false); + foreach (var wrapper in added) + { + RaiseProviderAdded(wrapper); + } + + // Reconcile out extensions whose directory no longer exists or no longer + // holds a valid manifest. + var accepted = DiscoverAcceptedManifests(ExtensionsPath); + List loadedDirectories; + lock (_extensionsLock) + { + loadedDirectories = _extensions.Select(e => e.ManifestDirectory).ToList(); + } + + var (_, toRemove) = ReconcileDirectories(accepted.Select(a => a.Directory), loadedDirectories); + foreach (var directory in toRemove) + { + var removed = await RemoveExtensionByDirectoryGatedAsync(directory).ConfigureAwait(false); + if (removed is not null) + { + RaiseProviderRemoved(removed); + } + } + + // Reload any still-present extension whose manifest changed on disk since it + // was loaded. A plain re-enumeration only adds/removes directories, so a manifest + // edit (new entry point, version, icon, and so on) would otherwise be ignored by + // an explicit refresh. + await ReloadChangedManifestsAsync(accepted).ConfigureAwait(false); + } + + return await GetInstalledExtensionsAsync(includeDisabledExtensions).ConfigureAwait(false); + } + + /// + /// Compares each currently loaded extension's manifest against the accepted manifest on + /// disk and hot-reloads any whose manifest changed. The caller passes the already + /// discovered/accepted set so the comparison uses the same duplicate-id policy as the + /// rest of the refresh. + /// + private async Task ReloadChangedManifestsAsync( + IReadOnlyList<(string Directory, JSExtensionManifest Manifest)> accepted) + { + List<(string Directory, JSExtensionManifest Loaded)> loaded; + lock (_extensionsLock) + { + loaded = _extensions + .Select(e => (e.ManifestDirectory, e.Manifest)) + .ToList(); + } + + foreach (var (directory, current) in accepted) + { + if (IsStopping(CancellationToken.None)) + { + break; + } + + var match = loaded.FirstOrDefault(l => PathsEqual(l.Directory, directory)); + if (match.Loaded is null) + { + continue; + } + + if (ManifestChanged(match.Loaded, current)) + { + Logger.LogInfo($"Refresh: manifest changed for {current.EffectiveDisplayName}; reloading."); + await HotReloadExtensionAsync(directory).ConfigureAwait(false); + } + } + } + + public IExtensionWrapper? GetInstalledExtension(string extensionUniqueId) + { + lock (_extensionsLock) + { + return _extensions.FirstOrDefault(e => e.ExtensionUniqueId == extensionUniqueId); + } + } + + public void EnableExtension(string extensionUniqueId) + { + lock (_extensionsLock) + { + _disabledExtensions.Remove(extensionUniqueId); + } + } + + public void DisableExtension(string extensionUniqueId) + { + lock (_extensionsLock) + { + _disabledExtensions.Add(extensionUniqueId); + } + } + + public void Dispose() + { + if (_disposed) + { + return; + } + + _disposed = true; + + _reload.Stop(); + StopDirectoryWatcher(); + StopAllSourceFileWatchers(); + _hotReloadDebouncer.Dispose(); + + List toDispose; + lock (_extensionsLock) + { + _shuttingDown = true; + toDispose = [.. _extensions]; + _extensions.Clear(); + _providerWrappers.Clear(); + _crashCounts.Clear(); + _providerIds.Clear(); + } + + foreach (var ext in toDispose) + { + ext.ProcessExited -= OnExtensionProcessExited; + ext.Dispose(); + } + + _notifications.Dispose(); + _directoryGate.Dispose(); + _reload.Dispose(); + } + + /// + /// Scans for subdirectories that contain a package.json with a + /// valid CmdPal manifest. Extracted as a static helper so discovery/manifest filtering + /// can be tested without spawning Node.js processes. + /// + /// The extensions root directory to scan. + /// The valid extensions found, as (directory, manifest) pairs. + internal static IReadOnlyList<(string Directory, JSExtensionManifest Manifest)> DiscoverManifests(string root) + { + var results = new List<(string, JSExtensionManifest)>(); + + if (string.IsNullOrEmpty(root) || !Directory.Exists(root)) + { + return results; + } + + string[] subdirectories; + try + { + subdirectories = Directory.GetDirectories(root); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + Logger.LogError($"Failed to enumerate JS extensions in {root}: {ex.Message}"); + return results; + } + + foreach (var subdir in subdirectories) + { + var manifestPath = Path.Combine(subdir, "package.json"); + if (!File.Exists(manifestPath)) + { + continue; + } + + var parseResult = JSExtensionManifest.TryParseFile(manifestPath); + if (!parseResult.IsValid || parseResult.Manifest is null) + { + Logger.LogDebug($"Skipping {subdir}: {parseResult.FailureReason}"); + continue; + } + + results.Add((subdir, parseResult.Manifest)); + } + + return results; + } + + /// + /// Applies the cross-extension duplicate-id policy to a discovered set: when two + /// extensions share a normalized name key, the one whose canonical directory path + /// sorts first (case-insensitive) wins and the rest are rejected. Sorting by path + /// makes the winner deterministic across runs regardless of filesystem enumeration + /// order. Extracted as a pure function so the policy can be tested directly. + /// + /// The discovered (directory, manifest) pairs. + /// The accepted pairs and the rejected pairs (with the winning directory). + internal static (IReadOnlyList<(string Directory, JSExtensionManifest Manifest)> Accepted, + IReadOnlyList<(string Directory, JSExtensionManifest Manifest, string WinnerDirectory)> Rejected) + ResolveIdCollisions(IReadOnlyList<(string Directory, JSExtensionManifest Manifest)> discovered) + { + var accepted = new List<(string, JSExtensionManifest)>(); + var rejected = new List<(string, JSExtensionManifest, string)>(); + var winners = new Dictionary(StringComparer.Ordinal); + + var ordered = discovered + .OrderBy(d => DirectoryLifecycleGate.Canonicalize(d.Directory), StringComparer.OrdinalIgnoreCase) + .ToList(); + + foreach (var (directory, manifest) in ordered) + { + var nameKey = manifest.NameKey; + if (string.IsNullOrEmpty(nameKey)) + { + accepted.Add((directory, manifest)); + continue; + } + + if (winners.TryGetValue(nameKey, out var winnerDirectory)) + { + rejected.Add((directory, manifest, winnerDirectory)); + } + else + { + winners[nameKey] = DirectoryLifecycleGate.Canonicalize(directory); + accepted.Add((directory, manifest)); + } + } + + return (accepted, rejected); + } + + /// + /// Computes the difference between what is currently discovered on disk and what is + /// currently loaded, using canonical case-insensitive directory comparison. Extracted + /// as a pure function so reconciliation can be tested without touching the filesystem. + /// + /// Directories discovered on disk. + /// Directories currently loaded by the service. + /// The directories to add (discovered but not loaded) and to remove (loaded but not discovered). + internal static (IReadOnlyList ToAdd, IReadOnlyList ToRemove) ReconcileDirectories( + IEnumerable discovered, + IEnumerable loaded) + { + var discoveredSet = new HashSet(discovered.Select(DirectoryLifecycleGate.Canonicalize), StringComparer.OrdinalIgnoreCase); + var loadedSet = new HashSet(loaded.Select(DirectoryLifecycleGate.Canonicalize), StringComparer.OrdinalIgnoreCase); + + var toAdd = discoveredSet.Where(d => !loadedSet.Contains(d)).ToList(); + var toRemove = loadedSet.Where(d => !discoveredSet.Contains(d)).ToList(); + + return (toAdd, toRemove); + } + + /// + /// Waits for a package's manifest to become parseable, retrying a bounded number of + /// times. This lets a slow or partially written install settle before it is loaded so + /// it is not loaded once, failed, and then never retried. Extracted with injectable + /// parse and delay callbacks so it can be tested deterministically. + /// + /// The package.json path to poll. + /// The maximum number of parse attempts. + /// Parses the manifest at a path. + /// Waits between attempts, given the zero-based attempt index. + /// Cancels the wait. + /// The parsed manifest, or null if it never became valid. + internal static async Task WaitForStableManifestAsync( + string manifestPath, + int attempts, + Func parse, + Func delay, + CancellationToken ct) + { + for (var attempt = 0; attempt < attempts; attempt++) + { + if (ct.IsCancellationRequested) + { + return null; + } + + var result = parse(manifestPath); + if (result.IsValid && result.Manifest is not null) + { + return result.Manifest; + } + + if (attempt < attempts - 1) + { + try + { + await delay(attempt, ct).ConfigureAwait(false); + } + catch (OperationCanceledException) + { + return null; + } + } + } + + return null; + } + + /// + /// Returns the immediate child directory of that contains + /// , i.e. the extension directory a changed path belongs + /// to. Returns null when the path is not under the root. Extracted as a pure helper + /// so it can be tested without a live watcher. + /// + /// The extensions root directory. + /// A path reported by the watcher. + /// The owning extension directory, or null. + internal static string? GetExtensionDirectoryForPath(string root, string fullPath) + { + if (string.IsNullOrEmpty(root) || string.IsNullOrEmpty(fullPath)) + { + return null; + } + + try + { + var normalizedRoot = Path.TrimEndingDirectorySeparator(Path.GetFullPath(root)); + var normalized = Path.TrimEndingDirectorySeparator(Path.GetFullPath(fullPath)); + + var prefix = normalizedRoot + Path.DirectorySeparatorChar; + if (!normalized.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)) + { + return null; + } + + var relative = normalized[prefix.Length..]; + var separatorIndex = relative.IndexOfAny([Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar]); + var firstSegment = separatorIndex < 0 ? relative : relative[..separatorIndex]; + if (string.IsNullOrEmpty(firstSegment)) + { + return null; + } + + return Path.Combine(normalizedRoot, firstSegment); + } + catch (Exception ex) when (ex is ArgumentException or NotSupportedException or PathTooLongException) + { + return null; + } + } + + /// + /// Returns true only when a watcher change under is a top-level + /// extension change: the extension directory itself (<root>/<extdir>) or its + /// own manifest (<root>/<extdir>/package.json). Anything deeper (a nested + /// package.json or a nested directory, for example under node_modules or a nested + /// package) returns false so the recursive root watcher does not treat it as an extension + /// upsert. Extracted as a pure helper so the depth filter can be tested without a live + /// watcher. + /// + /// The extensions root directory. + /// A path reported by the watcher. + /// True when the change belongs to a top-level extension entry. + internal static bool IsTopLevelExtensionChange(string root, string fullPath) + { + if (string.IsNullOrEmpty(root) || string.IsNullOrEmpty(fullPath)) + { + return false; + } + + try + { + var normalizedRoot = Path.TrimEndingDirectorySeparator(Path.GetFullPath(root)); + var normalized = Path.TrimEndingDirectorySeparator(Path.GetFullPath(fullPath)); + + var prefix = normalizedRoot + Path.DirectorySeparatorChar; + if (!normalized.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)) + { + return false; + } + + var relative = normalized[prefix.Length..]; + var segments = relative.Split( + [Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar], + StringSplitOptions.RemoveEmptyEntries); + + return segments.Length switch + { + // / (the extension directory created, renamed, or removed). + 1 => true, + + // //package.json (the extension's own manifest). + 2 => string.Equals(segments[1], "package.json", StringComparison.OrdinalIgnoreCase), + + // Anything deeper is a nested file or directory and is not an extension entry. + _ => false, + }; + } + catch (Exception ex) when (ex is ArgumentException or NotSupportedException or PathTooLongException) + { + return false; + } + } + + private static string GetDefaultExtensionsPath() + { + var localAppData = Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData); + return Path.Combine(localAppData, "Microsoft", "PowerToys", "CmdPal", "JSExtensions"); + } + + private static bool PathsEqual(string a, string b) => + string.Equals(Path.TrimEndingDirectorySeparator(a), Path.TrimEndingDirectorySeparator(b), StringComparison.OrdinalIgnoreCase); + + private static string CanonicalKey(string directory) => DirectoryLifecycleGate.Canonicalize(directory); + + private static bool IsManifestPath(string path) => + string.Equals(Path.GetFileName(path), "package.json", StringComparison.OrdinalIgnoreCase); + + private static bool IsWatchedSourceFile(string path) + { + var extension = Path.GetExtension(path); + foreach (var watched in WatchedSourceExtensions) + { + if (string.Equals(extension, watched, StringComparison.OrdinalIgnoreCase)) + { + return true; + } + } + + return false; + } + + /// + /// Returns true when any directory segment of is one the + /// watchers must ignore (for example node_modules or .git). This is a + /// segment-aware check, so a directory named "node_modules_backup" is not matched. + /// Extracted as a pure helper so it can be tested without a live watcher. + /// + /// The path reported by a watcher. + /// True when the path lies under an ignored directory segment. + internal static bool HasIgnoredDirectorySegment(string path) + { + if (string.IsNullOrEmpty(path)) + { + return false; + } + + var segments = path.Split( + [Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar], + StringSplitOptions.RemoveEmptyEntries); + + foreach (var segment in segments) + { + foreach (var ignored in IgnoredDirectorySegments) + { + if (string.Equals(segment, ignored, StringComparison.OrdinalIgnoreCase)) + { + return true; + } + } + } + + return false; + } + + /// + /// Returns true when a change to should trigger a + /// source hot-reload: it is a watched source file, is not filtered by the debouncer, + /// and is not under an ignored directory segment. Extracted as a pure helper so the + /// routing decision can be tested without a live watcher. + /// + /// The full path of the changed source file. + /// True when the change should trigger a hot-reload. + internal static bool ShouldReloadForSourceChange(string fullPath) => + !string.IsNullOrEmpty(fullPath) + && IsWatchedSourceFile(fullPath) + && HotReloadDebouncer.IsRelevantChange(fullPath) + && !HasIgnoredDirectorySegment(fullPath); + + /// + /// Discovers manifests and applies the duplicate-id collision policy, logging any + /// rejected duplicates. All full (re)load and reconciliation paths go through here so + /// they agree on the same deterministic winner. + /// + private static IReadOnlyList<(string Directory, JSExtensionManifest Manifest)> DiscoverAcceptedManifests(string root) + { + var discovered = DiscoverManifests(root); + var (accepted, rejected) = ResolveIdCollisions(discovered); + + foreach (var (directory, manifest, winnerDirectory) in rejected) + { + Logger.LogWarning( + $"Skipping JS extension at {directory}: duplicate id '{manifest.NameKey}' is already provided by {winnerDirectory}."); + } + + return accepted; + } + + private bool IsStopping(CancellationToken ct) => _disposed || _reload.IsStopRequested || ct.IsCancellationRequested; + + // Provider add/remove notifications are raised through a single ordered dispatcher so a + // consumer never observes an addition ahead of a removal that was raised before it. + private void RaiseProviderAdded(CommandProviderWrapper wrapper) => + _notifications.Enqueue(() => OnProviderAdded?.Invoke(this, [wrapper])); + + private void RaiseProviderRemoved(CommandProviderWrapper wrapper) => + _notifications.Enqueue(() => OnProviderRemoved?.Invoke(this, [wrapper])); + + // A swap (hot-reload or crash-restart) raises the removal of the old provider and the + // addition of the new one as one enqueued action so the pair can never be split, nor + // observed out of order, by another operation's emission. + private void RaiseProviderSwapped(CommandProviderWrapper? removed, CommandProviderWrapper added) => + _notifications.Enqueue(() => + { + if (removed is not null) + { + OnProviderRemoved?.Invoke(this, [removed]); + } + + OnProviderAdded?.Invoke(this, [added]); + }); + + private bool EnsureExtensionsDirectory() + { + if (Directory.Exists(ExtensionsPath)) + { + return true; + } + + try + { + Directory.CreateDirectory(ExtensionsPath); + Logger.LogDebug($"Created JS extensions directory: {ExtensionsPath}"); + return true; + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + Logger.LogError($"Failed to create JS extensions directory {ExtensionsPath}: {ex.Message}"); + return false; + } + } + + /// + /// Loads every discovered extension that is not already loaded, serialized per + /// directory through the lifecycle gate. Returns the wrappers that were added. + /// + private async Task> AddDiscoveredNotLoadedAsync(CancellationToken ct) + { + var added = new List(); + var accepted = DiscoverAcceptedManifests(ExtensionsPath); + + List loadedDirectories; + lock (_extensionsLock) + { + loadedDirectories = _extensions.Select(e => e.ManifestDirectory).ToList(); + } + + var (toAdd, _) = ReconcileDirectories(accepted.Select(a => a.Directory), loadedDirectories); + var toAddSet = new HashSet(toAdd, StringComparer.OrdinalIgnoreCase); + + foreach (var (directory, manifest) in accepted) + { + if (IsStopping(ct)) + { + break; + } + + if (!toAddSet.Contains(DirectoryLifecycleGate.Canonicalize(directory))) + { + continue; + } + + var wrapper = await AddExtensionGatedAsync(directory, manifest, ct).ConfigureAwait(false); + if (wrapper is not null) + { + added.Add(wrapper); + } + } + + return added; + } + + /// + /// Adds a single extension under its per-directory gate, skipping it if it is already + /// loaded. Serializing on the gate means a refresh, a watcher event, and a crash + /// restart for the same directory cannot launch duplicate processes. + /// + private async Task AddExtensionGatedAsync(string directory, JSExtensionManifest manifest, CancellationToken ct) + { + if (IsStopping(ct)) + { + return null; + } + + IDisposable gate; + try + { + gate = await _directoryGate.AcquireAsync(directory, ct).ConfigureAwait(false); + } + catch (OperationCanceledException) + { + return null; + } + catch (ObjectDisposedException) + { + return null; + } + + using (gate) + { + if (IsStopping(ct)) + { + return null; + } + + bool alreadyLoaded; + lock (_extensionsLock) + { + alreadyLoaded = _extensions.Any(e => PathsEqual(e.ManifestDirectory, directory)); + } + + if (alreadyLoaded) + { + return null; + } + + return await StartAndRegisterAsync(directory, manifest, resetCrashCount: true, ct).ConfigureAwait(false); + } + } + + /// + /// Starts an extension process and returns a started-but-unregistered instance, or null + /// if the process could not start or does not provide an . + /// The caller must hold the directory's lifecycle gate. This does not mutate the + /// service collections or reserve a provider id; it is the validate half of a + /// validate-then-swap so a hot-reload can start a replacement before removing the + /// incumbent. Any wrapper created here that fails validation is disposed so its process + /// is not leaked. + /// + private async Task StartInstanceAsync(string directory, JSExtensionManifest manifest, CancellationToken ct) + { + if (IsStopping(ct)) + { + return null; + } + + JSExtensionWrapper? extensionWrapper = null; + try + { + extensionWrapper = new JSExtensionWrapper(manifest, directory); + + await extensionWrapper.StartExtensionAsync().ConfigureAwait(false); + + if (!extensionWrapper.IsRunning()) + { + Logger.LogError($"Failed to start JS extension {manifest.EffectiveDisplayName}"); + extensionWrapper.SignalDispose(); + return null; + } + + var provider = await extensionWrapper.GetProviderAsync().ConfigureAwait(false); + if (provider is null) + { + Logger.LogWarning($"JS extension {manifest.EffectiveDisplayName} does not provide an ICommandProvider"); + extensionWrapper.SignalDispose(); + return null; + } + + // If shutdown started while we were spawning the process, discard the new + // extension rather than registering it after everything else has been torn down. + if (IsStopping(ct)) + { + extensionWrapper.SignalDispose(); + return null; + } + + var wrapper = new CommandProviderWrapper(extensionWrapper, provider, _taskScheduler); + return new StartedInstance(extensionWrapper, wrapper); + } + catch (Exception ex) + { + Logger.LogError($"Failed to load JS extension from {directory}: {ex.Message}"); + extensionWrapper?.SignalDispose(); + return null; + } + } + + /// + /// Starts an extension process and registers its provider. The caller must hold the + /// directory's lifecycle gate. Any wrapper created here that cannot be registered + /// (start failure, cancellation, shutdown, or a defensive duplicate) is disposed so its + /// process is not leaked. + /// + private async Task StartAndRegisterAsync(string directory, JSExtensionManifest manifest, bool resetCrashCount, CancellationToken ct) + { + var instance = await StartInstanceAsync(directory, manifest, ct).ConfigureAwait(false); + if (instance is null) + { + return null; + } + + var extensionWrapper = instance.Extension; + var wrapper = instance.Wrapper; + extensionWrapper.ProcessExited += OnExtensionProcessExited; + + var outcome = RegistrationOutcome.Added; + lock (_extensionsLock) + { + // Shutdown cleared the collections while this process was starting; do not + // register it, or its Node process and watcher would leak past shutdown. + if (_shuttingDown) + { + outcome = RegistrationOutcome.Stopping; + } + + // The per-directory gate prevents concurrent loads for one directory, but + // keep the incumbent and drop the newcomer defensively rather than leaking + // two live processes if a duplicate ever slips through. + else if (_extensions.Any(e => PathsEqual(e.ManifestDirectory, directory))) + { + outcome = RegistrationOutcome.DuplicateDirectory; + } + else if (!_providerIds.TryReserve(extensionWrapper.NameKey, CanonicalKey(directory))) + { + // Another directory already owns this provider id. Claiming the id and + // adding to _extensions happen as one atomic step under this lock, so no + // interleaving install, hot-reload, or crash-restart can register a + // second provider with the same id. + outcome = RegistrationOutcome.DuplicateId; + } + else + { + _extensions.Add(extensionWrapper); + _providerWrappers.Add(wrapper); + if (resetCrashCount) + { + _crashCounts.Remove(CanonicalKey(directory)); + } + } + } + + if (outcome != RegistrationOutcome.Added) + { + if (outcome == RegistrationOutcome.DuplicateId) + { + Logger.LogWarning( + $"Skipping JS extension at {directory}: provider id '{extensionWrapper.NameKey}' is already reserved by another extension."); + } + + extensionWrapper.ProcessExited -= OnExtensionProcessExited; + extensionWrapper.SignalDispose(); + return null; + } + + StartSourceFileWatcher(directory); + + // A process can exit immediately after init (for example a provider that faults + // on first use). If that exit fired before we subscribed to ProcessExited above, + // the event was missed; detect the dead process here and drive the same crash + // path so an immediate post-init crash is handled (restart or disable) instead + // of being registered as healthy. The handler runs on a separate task so it + // acquires the directory gate only after this registration releases it, and it is + // idempotent, so racing the real event is harmless. + if (!extensionWrapper.IsRunning()) + { + OnExtensionProcessExited(extensionWrapper, EventArgs.Empty); + } + + Logger.LogInfo($"Loaded JS extension: {manifest.EffectiveDisplayName}"); + return wrapper; + } + + /// + /// An extension that has started and validated (its process is running and it provides + /// an ) but has not yet been registered into the service + /// collections. Used by the validate-then-swap hot-reload path. + /// + private sealed record StartedInstance(JSExtensionWrapper Extension, CommandProviderWrapper Wrapper); + + private void OnExtensionProcessExited(object? sender, EventArgs e) + { + if (sender is JSExtensionWrapper wrapper) + { + _ = Task.Run(() => HandleExtensionCrashAsync(wrapper)); + } + } + + private async Task HandleExtensionCrashAsync(JSExtensionWrapper wrapper) + { + if (_disposed || _reload.IsStopRequested) + { + return; + } + + var directory = wrapper.ManifestDirectory; + + IDisposable gate; + try + { + gate = await _directoryGate.AcquireAsync(directory, _reload.Token).ConfigureAwait(false); + } + catch (OperationCanceledException) + { + return; + } + catch (ObjectDisposedException) + { + return; + } + + using (gate) + { + CommandProviderWrapper? removed; + int crashCount; + lock (_extensionsLock) + { + // The wrapper may already be gone (uninstall, hot-reload, or shutdown won the race). + if (!_extensions.Remove(wrapper)) + { + return; + } + + removed = _providerWrappers.FirstOrDefault(w => ReferenceEquals(w.Extension, wrapper)); + if (removed is not null) + { + _providerWrappers.Remove(removed); + } + + var key = CanonicalKey(directory); + _crashCounts.TryGetValue(key, out crashCount); + crashCount++; + _crashCounts[key] = crashCount; + + // Free the provider id as part of the same atomic removal so a different + // extension can claim it, and so the restart below can re-reserve it. + _providerIds.Release(wrapper.NameKey, key); + } + + wrapper.ProcessExited -= OnExtensionProcessExited; + + if (removed is not null) + { + RaiseProviderRemoved(removed); + } + + if (DecideCrashAction(crashCount, MaxRestartAttempts) == CrashAction.Disable) + { + Logger.LogError($"JS extension at {directory} crashed {crashCount} times consecutively; disabling it. Edit the source or reinstall to re-enable."); + + // Keep the source-file watcher alive so a developer source edit fires a + // hot-reload, which resets the crash count and retries the load. Stopping it + // here would strand the extension disabled until a full reinstall. + return; + } + + Logger.LogWarning($"JS extension at {directory} crashed (attempt {crashCount} of {MaxRestartAttempts}); restarting."); + + var manifestPath = Path.Combine(directory, "package.json"); + var parseResult = JSExtensionManifest.TryParseFile(manifestPath); + if (!parseResult.IsValid || parseResult.Manifest is null) + { + Logger.LogError($"Cannot restart JS extension at {directory}: {parseResult.FailureReason}"); + StopSourceFileWatcher(directory); + return; + } + + // Preserve the crash count across the restart so repeated crashes eventually disable it. + var restarted = await StartAndRegisterAsync(directory, parseResult.Manifest, resetCrashCount: false, _reload.Token).ConfigureAwait(false); + if (restarted is not null) + { + RaiseProviderAdded(restarted); + Logger.LogInfo($"Restarted JS extension: {parseResult.Manifest.EffectiveDisplayName}"); + } + } + } + + /// + /// Decides whether an extension that has just recorded its th + /// consecutive crash should be restarted or disabled. Extracted as a pure function so the + /// state transitions can be tested without spawning a Node.js process. + /// + /// The consecutive crash count, already incremented for this crash. + /// The maximum number of restart attempts allowed. + /// while at or below the limit; otherwise . + internal static CrashAction DecideCrashAction(int crashCount, int maxRestartAttempts) => + crashCount > maxRestartAttempts ? CrashAction.Disable : CrashAction.Restart; + + /// + /// Returns true when the salient fields of differ from + /// , i.e. an edit to the manifest that would change how the + /// extension runs or presents. Extracted as a pure function so an explicit refresh can + /// decide to reload a changed manifest without touching the filesystem in tests. + /// + /// The manifest the extension is currently running with. + /// The manifest as it now exists on disk. + /// True when the manifest changed in a way that warrants a reload. + internal static bool ManifestChanged(JSExtensionManifest loaded, JSExtensionManifest current) + { + if (loaded is null || current is null) + { + return false; + } + + return !string.Equals(loaded.Name, current.Name, StringComparison.Ordinal) + || !string.Equals(loaded.DisplayName, current.DisplayName, StringComparison.Ordinal) + || !string.Equals(loaded.Version, current.Version, StringComparison.Ordinal) + || !string.Equals(loaded.Description, current.Description, StringComparison.Ordinal) + || !string.Equals(loaded.Icon, current.Icon, StringComparison.Ordinal) + || !string.Equals(loaded.Publisher, current.Publisher, StringComparison.Ordinal) + || !string.Equals(loaded.Main, current.Main, StringComparison.Ordinal) + || !string.Equals(loaded.EntryPointPath, current.EntryPointPath, StringComparison.OrdinalIgnoreCase) + || loaded.Debug != current.Debug + || loaded.DebugPort != current.DebugPort; + } + + private void StartDirectoryWatcher() + { + if (_directoryWatcher is not null || !Directory.Exists(ExtensionsPath)) + { + return; + } + + try + { + _directoryWatcher = new FileSystemWatcher(ExtensionsPath) + { + // Observe both top-level directory changes and manifest files written + // (possibly late) inside a package, so a slow install or an atomic rename + // promotion is still discovered. + NotifyFilter = NotifyFilters.DirectoryName | NotifyFilters.FileName | NotifyFilters.LastWrite, + IncludeSubdirectories = true, + + // A recursive watch over the extensions root (which contains each + // extension's node_modules tree) can burst well past the default 8 KB + // buffer during an install. Enlarge it to make an overflow far less + // likely; the Error handler recovers if one still happens. + InternalBufferSize = 64 * 1024, + }; + + // Attach handlers before enabling events so a change that lands in the + // window between construction and subscription is not dropped. + _directoryWatcher.Created += OnDirectoryWatcherUpsert; + _directoryWatcher.Changed += OnDirectoryWatcherUpsert; + _directoryWatcher.Renamed += OnDirectoryWatcherRenamed; + _directoryWatcher.Deleted += OnDirectoryWatcherDeleted; + _directoryWatcher.Error += OnDirectoryWatcherError; + + _directoryWatcher.EnableRaisingEvents = true; + + Logger.LogDebug($"Started directory watcher for {ExtensionsPath}"); + } + catch (Exception ex) + { + Logger.LogError($"Failed to start directory watcher for {ExtensionsPath}: {ex.Message}"); + } + } + + private void StopDirectoryWatcher() + { + if (_directoryWatcher is null) + { + return; + } + + _directoryWatcher.Created -= OnDirectoryWatcherUpsert; + _directoryWatcher.Changed -= OnDirectoryWatcherUpsert; + _directoryWatcher.Renamed -= OnDirectoryWatcherRenamed; + _directoryWatcher.Deleted -= OnDirectoryWatcherDeleted; + _directoryWatcher.Error -= OnDirectoryWatcherError; + _directoryWatcher.Dispose(); + _directoryWatcher = null; + } + + private void OnDirectoryWatcherUpsert(object sender, FileSystemEventArgs e) + { + // Ignore churn under node_modules/.git (for example npm writing many package.json + // files during an install) so it cannot drive a discovery or hot-reload storm. + if (HasIgnoredDirectorySegment(e.FullPath)) + { + return; + } + + // The root watcher is recursive, so it also reports nested files and directories. + // Only a top-level / directory or its own //package.json + // is an extension change; a nested package.json or directory (a nested package or + // dependency) must not be treated as an extension upsert. + if (!IsTopLevelExtensionChange(ExtensionsPath, e.FullPath)) + { + return; + } + + // Only manifests and (newly created) directories drive discovery here; source + // file edits are handled by the per-extension source watcher. + if (IsManifestPath(e.FullPath) || Directory.Exists(e.FullPath)) + { + HandleDirectoryEntryUpsert(e.FullPath); + } + } + + private void OnDirectoryWatcherRenamed(object sender, RenamedEventArgs e) + { + // A rename can be an atomic promotion (temp -> final) or a demotion/uninstall + // (final -> temp). Treat the new name as a possible install and the old name as + // a possible removal, ignoring either side that sits under an ignored segment. + // The new name must also be a top-level extension entry (directory or its own + // manifest); a nested rename is not an extension change. + if (!HasIgnoredDirectorySegment(e.FullPath) + && IsTopLevelExtensionChange(ExtensionsPath, e.FullPath) + && (IsManifestPath(e.FullPath) || Directory.Exists(e.FullPath))) + { + HandleDirectoryEntryUpsert(e.FullPath); + } + + if (!HasIgnoredDirectorySegment(e.OldFullPath)) + { + HandleDirectoryEntryRemoved(e.OldFullPath); + } + } + + private void OnDirectoryWatcherDeleted(object sender, FileSystemEventArgs e) + { + if (HasIgnoredDirectorySegment(e.FullPath)) + { + return; + } + + HandleDirectoryEntryRemoved(e.FullPath); + } + + private void OnDirectoryWatcherError(object sender, ErrorEventArgs e) + { + var error = e.GetException(); + + // On an internal-buffer overflow the OS dropped an unknown set of events, so + // discovery would silently stop reflecting the extensions directory. Log it and + // run a full reconciliation to catch up on anything the watcher missed. Other + // errors (for example the directory going away) are logged for diagnosis. + Logger.LogError($"Directory watcher error for {ExtensionsPath}: {error.Message}"); + + if (error is InternalBufferOverflowException && !_disposed) + { + _ = Task.Run(async () => + { + try + { + await RefreshInstalledExtensionsAsync().ConfigureAwait(false); + } + catch (Exception ex) + { + Logger.LogError($"Failed to reconcile after directory watcher overflow: {ex.Message}"); + } + }); + } + } + + private void HandleDirectoryEntryUpsert(string changedPath) + { + var extensionDirectory = GetExtensionDirectoryForPath(ExtensionsPath, changedPath); + if (extensionDirectory is null) + { + return; + } + + var token = _reload.Token; + _ = Task.Run( + async () => + { + var manifest = await WaitForStableManifestInstanceAsync(extensionDirectory, token).ConfigureAwait(false); + if (manifest is null || _disposed || token.IsCancellationRequested) + { + return; + } + + bool alreadyLoaded; + lock (_extensionsLock) + { + alreadyLoaded = _extensions.Any(x => PathsEqual(x.ManifestDirectory, extensionDirectory)); + } + + if (alreadyLoaded) + { + // The manifest reappeared or changed for a loaded extension: reload it + // so the new manifest takes effect. + await HotReloadExtensionAsync(extensionDirectory).ConfigureAwait(false); + return; + } + + if (WouldCollideWithLoaded(extensionDirectory, manifest)) + { + Logger.LogWarning( + $"Skipping JS extension at {extensionDirectory}: an extension with id '{manifest.NameKey}' is already loaded."); + return; + } + + var wrapper = await AddExtensionGatedAsync(extensionDirectory, manifest, token).ConfigureAwait(false); + if (wrapper is not null) + { + RaiseProviderAdded(wrapper); + } + }, + token); + } + + private void HandleDirectoryEntryRemoved(string changedPath) + { + var extensionDirectory = GetExtensionDirectoryForPath(ExtensionsPath, changedPath); + if (extensionDirectory is null) + { + return; + } + + var token = _reload.Token; + _ = Task.Run( + async () => + { + // If the extension directory still holds a valid manifest, this was not a + // real uninstall (for example a temp file was removed); keep the extension. + var manifestPath = Path.Combine(extensionDirectory, "package.json"); + if (Directory.Exists(extensionDirectory) && File.Exists(manifestPath)) + { + return; + } + + var removed = await RemoveExtensionByDirectoryGatedAsync(extensionDirectory).ConfigureAwait(false); + if (removed is not null) + { + RaiseProviderRemoved(removed); + } + }, + token); + } + + private async Task WaitForStableManifestInstanceAsync(string directory, CancellationToken ct) + { + var manifestPath = Path.Combine(directory, "package.json"); + return await WaitForStableManifestAsync( + manifestPath, + ManifestStabilityAttempts, + JSExtensionManifest.TryParseFile, + (_, token) => Task.Delay(ManifestStabilityDelay, token), + ct).ConfigureAwait(false); + } + + /// + /// Returns true when loading from + /// would duplicate the id of an already-loaded extension coming from a different + /// directory. A full (re)load applies the path-sorted winner policy through + /// ; for a single dynamic install the already-loaded + /// extension is kept and the newcomer is rejected. + /// + private bool WouldCollideWithLoaded(string directory, JSExtensionManifest manifest) + { + var nameKey = manifest.NameKey; + if (string.IsNullOrEmpty(nameKey)) + { + return false; + } + + var canonical = DirectoryLifecycleGate.Canonicalize(directory); + lock (_extensionsLock) + { + return _extensions.Any(e => + string.Equals(e.NameKey, nameKey, StringComparison.Ordinal) && + !string.Equals(DirectoryLifecycleGate.Canonicalize(e.ManifestDirectory), canonical, StringComparison.OrdinalIgnoreCase)); + } + } + + private async Task RemoveExtensionByDirectoryGatedAsync(string directory) + { + IDisposable? gate = null; + try + { + gate = await _directoryGate.AcquireAsync(directory, CancellationToken.None).ConfigureAwait(false); + } + catch (ObjectDisposedException) + { + // The gate is being torn down; fall through and remove best-effort. + } + + try + { + return RemoveExtensionByDirectoryCore(directory); + } + finally + { + gate?.Dispose(); + + // Release the gate entry for the directory now that it is fully removed. + _directoryGate.Remove(directory); + } + } + + private CommandProviderWrapper? RemoveExtensionByDirectoryCore(string directory) + { + JSExtensionWrapper? extensionToRemove; + CommandProviderWrapper? wrapperToRemove; + + lock (_extensionsLock) + { + extensionToRemove = _extensions.FirstOrDefault(e => PathsEqual(e.ManifestDirectory, directory)); + if (extensionToRemove is null) + { + wrapperToRemove = null; + } + else + { + _extensions.Remove(extensionToRemove); + wrapperToRemove = _providerWrappers.FirstOrDefault(w => ReferenceEquals(w.Extension, extensionToRemove)); + if (wrapperToRemove is not null) + { + _providerWrappers.Remove(wrapperToRemove); + } + + _crashCounts.Remove(CanonicalKey(directory)); + _providerIds.Release(extensionToRemove.NameKey, CanonicalKey(directory)); + } + } + + // Always tear down the source watcher for the directory, even when no live + // extension matched (for example a crash-disabled extension that was already + // removed from the list but whose watcher was intentionally kept alive), so an + // uninstall never leaks a watcher. + StopSourceFileWatcher(directory); + + if (extensionToRemove is not null) + { + extensionToRemove.ProcessExited -= OnExtensionProcessExited; + extensionToRemove.SignalDispose(); + } + + return wrapperToRemove; + } + + private void StartSourceFileWatcher(string directory) + { + lock (_sourceWatcherLock) + { + if (_sourceFileWatchers.ContainsKey(directory)) + { + return; + } + + try + { + // Watch all files and filter to the source extensions in the handler so + // that .js, .mjs, and .cjs edits all trigger a hot-reload. + var watcher = new FileSystemWatcher(directory) + { + NotifyFilter = NotifyFilters.LastWrite | NotifyFilters.FileName, + IncludeSubdirectories = true, + + // A source extension recursively watches its own node_modules, so a + // dependency install can flood the default 8 KB buffer. Enlarge it and + // recover from any overflow in the Error handler. + InternalBufferSize = 64 * 1024, + }; + + watcher.Changed += OnSourceFileChanged; + watcher.Created += OnSourceFileChanged; + + // Editors commonly save atomically (write a temp file, then rename it over + // the target) and also delete/recreate files. Subscribe to Renamed and + // Deleted as well so those changes reload instead of being missed. + watcher.Renamed += OnSourceFileRenamed; + watcher.Deleted += OnSourceFileChanged; + watcher.Error += OnSourceWatcherError; + + // Enable events only after every handler is attached so an edit that lands + // during setup is not dropped. + watcher.EnableRaisingEvents = true; + + _sourceFileWatchers[directory] = watcher; + Logger.LogDebug($"Started source file watcher at {directory}"); + } + catch (Exception ex) + { + Logger.LogError($"Failed to start source file watcher at {directory}: {ex.Message}"); + } + } + } + + private void StopSourceFileWatcher(string directory) + { + lock (_sourceWatcherLock) + { + if (_sourceFileWatchers.TryGetValue(directory, out var watcher)) + { + watcher.Changed -= OnSourceFileChanged; + watcher.Created -= OnSourceFileChanged; + watcher.Renamed -= OnSourceFileRenamed; + watcher.Deleted -= OnSourceFileChanged; + watcher.Error -= OnSourceWatcherError; + watcher.Dispose(); + _sourceFileWatchers.Remove(directory); + } + } + + _hotReloadDebouncer.Cancel(directory); + } + + private void StopAllSourceFileWatchers() + { + lock (_sourceWatcherLock) + { + foreach (var watcher in _sourceFileWatchers.Values) + { + watcher.Changed -= OnSourceFileChanged; + watcher.Created -= OnSourceFileChanged; + watcher.Renamed -= OnSourceFileRenamed; + watcher.Deleted -= OnSourceFileChanged; + watcher.Error -= OnSourceWatcherError; + watcher.Dispose(); + } + + _sourceFileWatchers.Clear(); + } + + // Advance the debounce generation so a pending hot-reload callback that was already + // queued before this stop is dropped instead of firing against the next load cycle. + _hotReloadDebouncer.CancelAll(); + } + + private void OnSourceFileChanged(object sender, FileSystemEventArgs e) + { + RouteSourceChange(e.FullPath); + } + + private void OnSourceFileRenamed(object sender, RenamedEventArgs e) + { + // An atomic save writes a temp file and renames it over the target, so the new + // path is the real source file. Route both the new and old paths so a rename into + // or out of a watched source name reloads. + RouteSourceChange(e.FullPath); + RouteSourceChange(e.OldFullPath); + } + + private void OnSourceWatcherError(object sender, ErrorEventArgs e) + { + var error = e.GetException(); + + // Find the directory this watcher belongs to so the recovery targets the right + // extension. The watcher instance is the dictionary value, so match on reference. + string? directory = null; + lock (_sourceWatcherLock) + { + foreach (var pair in _sourceFileWatchers) + { + if (ReferenceEquals(pair.Value, sender)) + { + directory = pair.Key; + break; + } + } + } + + Logger.LogError($"Source file watcher error for {directory ?? "(unknown)"}: {error.Message}"); + + // A buffer overflow dropped an unknown set of edits, so the extension may be stale. + // Queue a hot reload of the affected directory to pick up the current on-disk state. + if (error is InternalBufferOverflowException && directory is not null && !_disposed) + { + _hotReloadDebouncer.Notify(directory, directory); + } + } + + private void RouteSourceChange(string fullPath) + { + if (!ShouldReloadForSourceChange(fullPath)) + { + return; + } + + var directory = FindWatchedDirectory(fullPath); + if (directory is not null) + { + _hotReloadDebouncer.Notify(directory, fullPath); + } + } + + private string? FindWatchedDirectory(string changedPath) + { + lock (_sourceWatcherLock) + { + foreach (var directory in _sourceFileWatchers.Keys) + { + if (IsUnderDirectory(changedPath, directory)) + { + return directory; + } + } + } + + return null; + } + + /// + /// Returns a value indicating whether is + /// itself or a descendant of it, matching only on a directory boundary. A plain prefix check + /// would treat "foo-bar" as being under "foo"; this does not. + /// + /// The candidate path (typically a changed file). + /// The directory to test containment against. + /// True when equals or sits under . + internal static bool IsUnderDirectory(string path, string directory) + { + if (string.IsNullOrEmpty(path) || string.IsNullOrEmpty(directory)) + { + return false; + } + + string normalizedPath; + string normalizedDir; + try + { + normalizedPath = Path.TrimEndingDirectorySeparator(Path.GetFullPath(path)); + normalizedDir = Path.TrimEndingDirectorySeparator(Path.GetFullPath(directory)); + } + catch (Exception ex) when (ex is ArgumentException or NotSupportedException or PathTooLongException) + { + return false; + } + + if (normalizedPath.Length == normalizedDir.Length) + { + return string.Equals(normalizedPath, normalizedDir, StringComparison.OrdinalIgnoreCase); + } + + return normalizedPath.Length > normalizedDir.Length + && normalizedPath.StartsWith(normalizedDir, StringComparison.OrdinalIgnoreCase) + && (normalizedPath[normalizedDir.Length] == Path.DirectorySeparatorChar + || normalizedPath[normalizedDir.Length] == Path.AltDirectorySeparatorChar); + } + + private async Task HotReloadExtensionAsync(string directory) + { + if (_disposed || _reload.IsStopRequested) + { + return; + } + + var manifestPath = Path.Combine(directory, "package.json"); + var parseResult = JSExtensionManifest.TryParseFile(manifestPath); + if (!parseResult.IsValid || parseResult.Manifest is null) + { + Logger.LogWarning($"Skipping hot-reload for {directory}: {parseResult.FailureReason}"); + return; + } + + Logger.LogInfo($"Hot-reload: restarting {parseResult.Manifest.EffectiveDisplayName}"); + + IDisposable gate; + try + { + gate = await _directoryGate.AcquireAsync(directory, _reload.Token).ConfigureAwait(false); + } + catch (OperationCanceledException) + { + return; + } + catch (ObjectDisposedException) + { + return; + } + + using (gate) + { + // Validate-then-swap. Start the replacement FIRST, before removing the + // incumbent, so a failed reload keeps the incumbent provider (and its source + // watcher) live and a later corrective edit re-triggers this reload. The old + // provider is only removed once the new one has started and registered, so a + // duplicate-id refresh never leaves the directory with neither provider. + var replacement = await StartInstanceAsync(directory, parseResult.Manifest, _reload.Token).ConfigureAwait(false); + if (replacement is null) + { + Logger.LogError( + $"Hot-reload failed: {parseResult.Manifest.EffectiveDisplayName} did not restart; keeping the current instance."); + return; + } + + var newExtension = replacement.Extension; + var newWrapper = replacement.Wrapper; + newExtension.ProcessExited += OnExtensionProcessExited; + + JSExtensionWrapper? incumbentExtension = null; + CommandProviderWrapper? removedWrapper = null; + var swapped = false; + var key = CanonicalKey(directory); + + lock (_extensionsLock) + { + if (!_shuttingDown) + { + // Remove the incumbent from the collections and release its provider id + // so the replacement can claim it. + incumbentExtension = _extensions.FirstOrDefault(e => PathsEqual(e.ManifestDirectory, directory)); + if (incumbentExtension is not null) + { + _extensions.Remove(incumbentExtension); + removedWrapper = _providerWrappers.FirstOrDefault(w => ReferenceEquals(w.Extension, incumbentExtension)); + if (removedWrapper is not null) + { + _providerWrappers.Remove(removedWrapper); + } + + _providerIds.Release(incumbentExtension.NameKey, key); + } + + if (_providerIds.TryReserve(newExtension.NameKey, key)) + { + _extensions.Add(newExtension); + _providerWrappers.Add(newWrapper); + _crashCounts.Remove(key); + swapped = true; + } + else if (incumbentExtension is not null) + { + // The replacement's provider id is owned by a different directory. + // Restore the incumbent so the reload does not lose both providers. + _providerIds.TryReserve(incumbentExtension.NameKey, key); + _extensions.Add(incumbentExtension); + if (removedWrapper is not null) + { + _providerWrappers.Add(removedWrapper); + } + } + } + } + + if (!swapped) + { + // The swap did not happen (shutdown, or the new provider id collided). + // Tear down the freshly started replacement and keep the incumbent, which + // is either still registered (restored above) or being torn down by + // shutdown. Its source watcher was never stopped, so a later edit reloads. + newExtension.ProcessExited -= OnExtensionProcessExited; + newExtension.SignalDispose(); + Logger.LogWarning( + $"Hot-reload for {parseResult.Manifest.EffectiveDisplayName} could not register the new instance; keeping the previous one."); + return; + } + + // The source watcher for this directory was never stopped, so it is preserved + // across the reload; ensure one exists for the case where the incumbent had none. + StartSourceFileWatcher(directory); + + // Dispose the incumbent only after the replacement is registered, so there is + // never a window with no provider for this directory. + if (incumbentExtension is not null) + { + incumbentExtension.ProcessExited -= OnExtensionProcessExited; + incumbentExtension.SignalDispose(); + } + + // Emit the removal and addition as a single ordered pair so consumers observe + // the swap in a consistent order. + RaiseProviderSwapped(removedWrapper, newWrapper); + + // Catch an immediate post-init exit the same way the initial registration does. + if (!newExtension.IsRunning()) + { + OnExtensionProcessExited(newExtension, EventArgs.Empty); + } + + Logger.LogInfo($"Hot-reload completed for {parseResult.Manifest.EffectiveDisplayName}"); + } + } +} diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Services/NodeRuntimeLocator.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Services/NodeRuntimeLocator.cs new file mode 100644 index 0000000000..85dfa79936 --- /dev/null +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Services/NodeRuntimeLocator.cs @@ -0,0 +1,71 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Collections.Generic; +using System.IO; + +namespace Microsoft.CmdPal.UI.ViewModels.Services; + +/// +/// Resolves an absolute path to the Node.js runtime (node.exe) by probing the +/// process PATH. Launching an explicit, validated absolute path rather than the bare +/// name node keeps from resolving +/// node.exe out of the spawning process's working directory (which for a JS +/// extension is the extension's own, untrusted, folder) or via other implicit search +/// locations. It also lets the caller surface a specific "Node.js not found" error +/// instead of a generic Win32 launch failure. +/// +internal static class NodeRuntimeLocator +{ + private const string NodeExecutableName = "node.exe"; + + /// + /// Resolves node.exe from the current process PATH. + /// + /// The absolute path to node.exe, or when it is not on PATH. + internal static string? ResolveNodeExecutable() => ResolveNodeExecutable(GetPathDirectories()); + + /// + /// Resolves node.exe from an explicit ordered list of directories. Exposed for testing. + /// + /// The directories to probe, in priority order. + /// The absolute path to the first existing node.exe, or . + internal static string? ResolveNodeExecutable(IReadOnlyList pathDirectories) + { + ArgumentNullException.ThrowIfNull(pathDirectories); + + foreach (var directory in pathDirectories) + { + string candidate; + try + { + candidate = Path.Combine(directory, NodeExecutableName); + } + catch (ArgumentException) + { + // Malformed PATH entry; skip it. + continue; + } + + if (File.Exists(candidate)) + { + return candidate; + } + } + + return null; + } + + private static IReadOnlyList GetPathDirectories() + { + var pathVariable = Environment.GetEnvironmentVariable("PATH"); + if (string.IsNullOrEmpty(pathVariable)) + { + return []; + } + + return pathVariable.Split(Path.PathSeparator, StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries); + } +} diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Services/ProviderIdReservations.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Services/ProviderIdReservations.cs new file mode 100644 index 0000000000..c56bde9056 --- /dev/null +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Services/ProviderIdReservations.cs @@ -0,0 +1,95 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Collections.Generic; +using System.Threading; + +namespace Microsoft.CmdPal.UI.ViewModels.Services; + +/// +/// A synchronized registry of provider ids (normalized manifest name keys) that are +/// currently claimed by a loaded extension, keyed to the canonical directory that owns +/// them. Every registration path (initial scan, refresh, dynamic install, hot-reload, +/// and crash-restart) consults and claims the same registry as one atomic step, so two +/// extensions can never register the same provider id regardless of how they arrive. +/// +/// +/// The registry keeps its own lock so it is independently thread-safe. The service also +/// invokes it while holding its extensions lock so that the id claim and the in-memory +/// extension list stay consistent as a single atomic operation. +/// +internal sealed class ProviderIdReservations +{ + private readonly Lock _lock = new(); + + // Provider id (ordinal name key) -> canonical directory that owns it. + private readonly Dictionary _owners = new(StringComparer.Ordinal); + + /// + /// Atomically claims for . + /// An empty provider id is never reserved (there is nothing to collide on). + /// + /// The normalized provider id (manifest name key). + /// The canonical directory attempting to own the id. + /// + /// True when the id is now owned by (either newly + /// claimed or already owned by the same directory); false when a different directory + /// already owns it. + /// + public bool TryReserve(string? providerId, string canonicalDirectory) + { + if (string.IsNullOrEmpty(providerId)) + { + return true; + } + + lock (_lock) + { + if (_owners.TryGetValue(providerId, out var owner)) + { + return string.Equals(owner, canonicalDirectory, StringComparison.OrdinalIgnoreCase); + } + + _owners[providerId] = canonicalDirectory; + return true; + } + } + + /// + /// Releases only when it is currently owned by + /// , so a stale release from a different owner + /// cannot free an id that has since been claimed by someone else. + /// + /// The normalized provider id (manifest name key). + /// The canonical directory releasing the id. + public void Release(string? providerId, string canonicalDirectory) + { + if (string.IsNullOrEmpty(providerId)) + { + return; + } + + lock (_lock) + { + if (_owners.TryGetValue(providerId, out var owner) && + string.Equals(owner, canonicalDirectory, StringComparison.OrdinalIgnoreCase)) + { + _owners.Remove(providerId); + } + } + } + + /// + /// Drops every reservation. Used when the service stops or is disposed and all + /// extensions are torn down together. + /// + public void Clear() + { + lock (_lock) + { + _owners.Clear(); + } + } +} diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Services/ReloadCancellation.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Services/ReloadCancellation.cs new file mode 100644 index 0000000000..83f4288fdb --- /dev/null +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Services/ReloadCancellation.cs @@ -0,0 +1,129 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Threading; + +namespace Microsoft.CmdPal.UI.ViewModels.Services; + +/// +/// A cancellation source that can be reused across successive service load cycles. +/// A single can only transition to canceled +/// once, so a service that shares one token between "stop" and "load again" would +/// keep handing out an already-canceled token after the first stop. This wrapper +/// hands out the live token, lets callers request a stop, and swaps in a fresh +/// source when a new load cycle begins, disposing the previous one safely. +/// +internal sealed partial class ReloadCancellation : IDisposable +{ + private readonly Lock _lock = new(); + private CancellationTokenSource _cts = new(); + private bool _disposed; + + /// + /// Gets the token for the current load cycle. Once the wrapper has been stopped + /// or disposed the returned token is already canceled, so callers observe the + /// stop request without touching a disposed source. + /// + public CancellationToken Token + { + get + { + lock (_lock) + { + if (_disposed || _cts.IsCancellationRequested) + { + return new CancellationToken(canceled: true); + } + + return _cts.Token; + } + } + } + + /// + /// Gets a value indicating whether a stop (or dispose) has been requested for the + /// current cycle. New work should bail out when this is true. + /// + public bool IsStopRequested + { + get + { + lock (_lock) + { + return _disposed || _cts.IsCancellationRequested; + } + } + } + + /// + /// Ensures a fresh, uncanceled token is available for a new load cycle. When the + /// current source has already been canceled it is disposed and replaced. Returns + /// the token for the new cycle. After the wrapper has been disposed this returns + /// an already-canceled token instead of throwing. + /// + /// The token that governs the newly started cycle. + public CancellationToken BeginCycle() + { + lock (_lock) + { + if (_disposed) + { + return new CancellationToken(canceled: true); + } + + if (_cts.IsCancellationRequested) + { + _cts.Dispose(); + _cts = new CancellationTokenSource(); + } + + return _cts.Token; + } + } + + /// + /// Requests cancellation of the current cycle without disposing the source, so + /// in-flight callers that already captured the token observe the cancellation. + /// The source is replaced on the next . + /// + public void Stop() + { + lock (_lock) + { + if (_disposed) + { + return; + } + + if (!_cts.IsCancellationRequested) + { + _cts.Cancel(); + } + } + } + + public void Dispose() + { + lock (_lock) + { + if (_disposed) + { + return; + } + + _disposed = true; + + try + { + _cts.Cancel(); + } + catch (ObjectDisposedException) + { + } + + _cts.Dispose(); + } + } +} diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Services/SerialNotificationDispatcher.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Services/SerialNotificationDispatcher.cs new file mode 100644 index 0000000000..1f11305cbf --- /dev/null +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Services/SerialNotificationDispatcher.cs @@ -0,0 +1,105 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Threading.Channels; +using System.Threading.Tasks; +using ManagedCommon; + +namespace Microsoft.CmdPal.UI.ViewModels.Services; + +/// +/// A single ordered dispatch path for provider add/remove notifications. Every emission +/// is enqueued and run by one worker in strict first-in-first-out order, so a consumer +/// can never observe a provider addition before the removal that was enqueued ahead of +/// it, even when the two originate on different threads (a hot-reload swap, a crash +/// restart, an install, and an uninstall can all race). +/// +/// +/// The service raises the paired removal and addition of a hot-reload or crash-restart as +/// a single enqueued action so the pair is never split by another operation's emission. +/// Because a slow consumer handler runs on the worker rather than the caller, an emission +/// cannot deadlock or reorder against the operation that produced it. +/// +internal sealed class SerialNotificationDispatcher : IDisposable +{ + private readonly Channel _queue = Channel.CreateUnbounded( + new UnboundedChannelOptions + { + SingleReader = true, + AllowSynchronousContinuations = false, + }); + + private readonly Task _worker; + private bool _disposed; + + public SerialNotificationDispatcher() + { + _worker = Task.Run(RunAsync); + } + + /// + /// Enqueues a notification to be raised on the worker after every notification already + /// enqueued. Dropped silently once the dispatcher has been disposed. + /// + /// The emission to run in order. + public void Enqueue(Action notification) + { + ArgumentNullException.ThrowIfNull(notification); + + if (_disposed) + { + return; + } + + _queue.Writer.TryWrite(notification); + } + + public void Dispose() + { + if (_disposed) + { + return; + } + + _disposed = true; + + // Stop accepting new notifications and let the worker drain to completion so a + // removal already queued ahead of an addition is not stranded. + _queue.Writer.TryComplete(); + + try + { + _worker.Wait(TimeSpan.FromSeconds(2)); + } + catch (AggregateException) + { + // The worker swallows handler exceptions; nothing actionable here. + } + } + + private async Task RunAsync() + { + try + { + while (await _queue.Reader.WaitToReadAsync().ConfigureAwait(false)) + { + while (_queue.Reader.TryRead(out var notification)) + { + try + { + notification(); + } + catch (Exception ex) + { + Logger.LogError($"A provider notification handler threw: {ex.Message}"); + } + } + } + } + catch (ChannelClosedException) + { + } + } +} diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/App.xaml.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI/App.xaml.cs index 15e6ee4d07..f31e8adfd9 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/App.xaml.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/App.xaml.cs @@ -271,6 +271,7 @@ public partial class App : Application, IDisposable // Load IExtensionServices here services.AddSingleton(); services.AddSingleton(); + services.AddSingleton(); services.AddSingleton(); diff --git a/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/DirectoryLifecycleGateTests.cs b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/DirectoryLifecycleGateTests.cs new file mode 100644 index 0000000000..e969008ae0 --- /dev/null +++ b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/DirectoryLifecycleGateTests.cs @@ -0,0 +1,206 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.IO; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CmdPal.UI.ViewModels.Services; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Microsoft.CmdPal.UI.ViewModels.UnitTests; + +/// +/// Verifies the per-directory lifecycle gate (p4-02). Concurrent triggers (initial +/// load, refresh, crash-restart, hot-reload) must be serialized per canonical +/// directory so they cannot launch duplicate processes, while different directories +/// still run concurrently. Removing an entry during a concurrent acquire must not +/// throw an ObjectDisposedException. +/// +[TestClass] +public class DirectoryLifecycleGateTests +{ + [TestMethod] + public void Canonicalize_TrailingSeparatorAndCase_ProduceSameKey() + { + var a = DirectoryLifecycleGate.Canonicalize(@"C:\temp\Ext"); + var b = DirectoryLifecycleGate.Canonicalize(@"C:\temp\Ext\"); + Assert.AreEqual(a, b); + } + + [TestMethod] + public async Task AcquireAsync_SameDirectory_SerializesOperations() + { + using var gate = new DirectoryLifecycleGate(); + const string Dir = @"C:\temp\extension-a"; + + var running = 0; + var maxConcurrent = 0; + var sync = new object(); + + async Task Operation() + { + using (await gate.AcquireAsync(Dir, CancellationToken.None)) + { + lock (sync) + { + running++; + maxConcurrent = Math.Max(maxConcurrent, running); + } + + await Task.Delay(25); + + lock (sync) + { + running--; + } + } + } + + await Task.WhenAll(Operation(), Operation(), Operation(), Operation()); + + Assert.AreEqual(1, maxConcurrent, "Operations for one directory must never overlap."); + } + + [TestMethod] + public async Task AcquireAsync_DifferentDirectories_RunConcurrently() + { + using var gate = new DirectoryLifecycleGate(); + + var firstEntered = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var releaseFirst = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + var firstTask = Task.Run(async () => + { + using (await gate.AcquireAsync(@"C:\temp\dir-1", CancellationToken.None)) + { + firstEntered.SetResult(); + await releaseFirst.Task; + } + }); + + await firstEntered.Task.WaitAsync(TimeSpan.FromSeconds(5)); + + // A different directory must be acquirable while the first is still held. + using (await gate.AcquireAsync(@"C:\temp\dir-2", CancellationToken.None).WaitAsync(TimeSpan.FromSeconds(5))) + { + Assert.IsTrue(true, "Acquired a second directory while the first was held."); + } + + releaseFirst.SetResult(); + await firstTask.WaitAsync(TimeSpan.FromSeconds(5)); + } + + [TestMethod] + public async Task Remove_DuringConcurrentAcquire_DoesNotThrow() + { + using var gate = new DirectoryLifecycleGate(); + const string Dir = @"C:\temp\removed-dir"; + + using (await gate.AcquireAsync(Dir, CancellationToken.None)) + { + // Removing the entry while it is held marks it for removal; the release + // that follows must not throw even though the entry was removed. + gate.Remove(Dir); + } + + // Re-acquiring after a removal transparently creates a fresh entry. + using (await gate.AcquireAsync(Dir, CancellationToken.None)) + { + Assert.IsTrue(true); + } + } + + [TestMethod] + public async Task Remove_WhileHeld_NewAcquireSerializesBehindPriorGeneration() + { + using var gate = new DirectoryLifecycleGate(); + const string Dir = @"C:\temp\overlap-dir"; + + var aEntered = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var releaseA = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var cEntered = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + var aTask = Task.Run(async () => + { + using (await gate.AcquireAsync(Dir, CancellationToken.None)) + { + aEntered.SetResult(); + await releaseA.Task; + } + }); + + await aEntered.Task.WaitAsync(TimeSpan.FromSeconds(5)); + + // Remove the directory while the prior generation (A) still holds it. A new + // generation must strictly supersede it, never overlap it. + gate.Remove(Dir); + + var cTask = Task.Run(async () => + { + using (await gate.AcquireAsync(Dir, CancellationToken.None)) + { + cEntered.SetResult(); + } + }); + + // C must not enter while A still holds the gate, even though Remove was called in + // between. Before the fix, Remove evicted the entry immediately, so C would create + // a fresh entry with a new semaphore and run concurrently with A. + var enteredEarly = await Task.WhenAny(cEntered.Task, Task.Delay(200)) == cEntered.Task; + Assert.IsFalse(enteredEarly, "A new generation after Remove must serialize behind the still-live prior generation."); + + releaseA.SetResult(); + await cEntered.Task.WaitAsync(TimeSpan.FromSeconds(5)); + await Task.WhenAll(aTask, cTask).WaitAsync(TimeSpan.FromSeconds(5)); + } + + [TestMethod] + public async Task Remove_WhileHeld_OverlappingCycles_NeverRunConcurrently() + { + using var gate = new DirectoryLifecycleGate(); + const string Dir = @"C:\temp\overlap-cycles"; + + var running = 0; + var maxConcurrent = 0; + var sync = new object(); + + async Task Cycle() + { + using (await gate.AcquireAsync(Dir, CancellationToken.None)) + { + lock (sync) + { + running++; + maxConcurrent = Math.Max(maxConcurrent, running); + } + + await Task.Delay(15); + + // Marking the directory for removal mid-cycle starts a fresh generation for + // any queued acquire; it must still not overlap this one. + gate.Remove(Dir); + + lock (sync) + { + running--; + } + } + } + + await Task.WhenAll(Cycle(), Cycle(), Cycle(), Cycle()).WaitAsync(TimeSpan.FromSeconds(10)); + + Assert.AreEqual(1, maxConcurrent, "Overlapping begin/complete cycles for one directory must never run concurrently."); + } + + [TestMethod] + public async Task AcquireAsync_AfterDispose_Throws() + { + var gate = new DirectoryLifecycleGate(); + gate.Dispose(); + + await Assert.ThrowsExceptionAsync( + async () => await gate.AcquireAsync(@"C:\temp\any", CancellationToken.None)); + } +} diff --git a/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/HotReloadDebouncerTests.cs b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/HotReloadDebouncerTests.cs new file mode 100644 index 0000000000..0ca0060aad --- /dev/null +++ b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/HotReloadDebouncerTests.cs @@ -0,0 +1,136 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Threading; +using Microsoft.CmdPal.UI.ViewModels.Services; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Microsoft.CmdPal.UI.ViewModels.UnitTests; + +[TestClass] +public class HotReloadDebouncerTests +{ + [TestMethod] + public void IsRelevantChange_NodeModulesPath_IsIgnored() + { + Assert.IsFalse(HotReloadDebouncer.IsRelevantChange(@"C:\ext\node_modules\pkg\index.js")); + Assert.IsFalse(HotReloadDebouncer.IsRelevantChange(@"C:\ext\NODE_MODULES\pkg\index.js")); + Assert.IsFalse(HotReloadDebouncer.IsRelevantChange(string.Empty)); + } + + [TestMethod] + public void IsRelevantChange_SourceFile_IsRelevant() + { + Assert.IsTrue(HotReloadDebouncer.IsRelevantChange(@"C:\ext\dist\index.js")); + } + + [TestMethod] + public void Notify_RapidChanges_InvokesCallbackOnce() + { + var fired = new CountdownEvent(1); + var count = 0; + using var debouncer = new HotReloadDebouncer( + _ => + { + Interlocked.Increment(ref count); + fired.Signal(); + }, + TimeSpan.FromMilliseconds(120)); + + // Simulate a burst of saves well within the debounce window. + for (var i = 0; i < 8; i++) + { + debouncer.Notify(@"C:\ext", @"C:\ext\dist\index.js"); + Thread.Sleep(10); + } + + Assert.IsTrue(fired.Wait(TimeSpan.FromSeconds(2)), "Callback was not invoked."); + + // Give any (erroneous) extra timers a chance to fire before asserting. + Thread.Sleep(200); + Assert.AreEqual(1, Volatile.Read(ref count)); + } + + [TestMethod] + public void Notify_NodeModulesChange_DoesNotInvokeCallback() + { + var count = 0; + using var debouncer = new HotReloadDebouncer( + _ => Interlocked.Increment(ref count), + TimeSpan.FromMilliseconds(80)); + + debouncer.Notify(@"C:\ext", @"C:\ext\node_modules\pkg\index.js"); + + Thread.Sleep(250); + Assert.AreEqual(0, Volatile.Read(ref count)); + } + + [TestMethod] + public void Notify_DistinctKeys_InvokesCallbackPerKey() + { + var fired = new CountdownEvent(2); + using var debouncer = new HotReloadDebouncer( + _ => fired.Signal(), + TimeSpan.FromMilliseconds(80)); + + debouncer.Notify(@"C:\ext-a", @"C:\ext-a\index.js"); + debouncer.Notify(@"C:\ext-b", @"C:\ext-b\index.js"); + + Assert.IsTrue(fired.Wait(TimeSpan.FromSeconds(2)), "Both keys should have fired."); + } + + [TestMethod] + public void Cancel_PreventsPendingCallback() + { + var count = 0; + using var debouncer = new HotReloadDebouncer( + _ => Interlocked.Increment(ref count), + TimeSpan.FromMilliseconds(150)); + + debouncer.Notify(@"C:\ext", @"C:\ext\index.js"); + debouncer.Cancel(@"C:\ext"); + + Thread.Sleep(300); + Assert.AreEqual(0, Volatile.Read(ref count)); + } + + // r3-p4-09: a debounce pending when the service stops between load generations must + // not fire against the next generation. CancelAll advances the generation and cancels + // every pending timer, so a callback armed before the stop is dropped. + [TestMethod] + public void CancelAll_DropsPendingCallbacksAcrossKeys() + { + var count = 0; + using var debouncer = new HotReloadDebouncer( + _ => Interlocked.Increment(ref count), + TimeSpan.FromMilliseconds(150)); + + debouncer.Notify(@"C:\ext-a", @"C:\ext-a\index.js"); + debouncer.Notify(@"C:\ext-b", @"C:\ext-b\index.js"); + debouncer.CancelAll(); + + Thread.Sleep(350); + Assert.AreEqual(0, Volatile.Read(ref count), "Pending callbacks must not fire after CancelAll."); + } + + // r3-p4-09: after a stop (CancelAll), a fresh Notify belongs to the new generation and + // must still fire normally, proving CancelAll cancels the prior generation only. + [TestMethod] + public void CancelAll_DoesNotBlockLaterGeneration() + { + var fired = new CountdownEvent(1); + using var debouncer = new HotReloadDebouncer( + _ => fired.Signal(), + TimeSpan.FromMilliseconds(100)); + + debouncer.Notify(@"C:\ext", @"C:\ext\index.js"); + debouncer.CancelAll(); + + // A change notified after the stop starts a new generation and must fire. + debouncer.Notify(@"C:\ext", @"C:\ext\index.js"); + + Assert.IsTrue(fired.Wait(TimeSpan.FromSeconds(2)), "A change after CancelAll should still reload."); + } +} diff --git a/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JSCommandProviderProxyFrozenTests.cs b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JSCommandProviderProxyFrozenTests.cs new file mode 100644 index 0000000000..559331f9d3 --- /dev/null +++ b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JSCommandProviderProxyFrozenTests.cs @@ -0,0 +1,78 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Text.Json; +using Microsoft.CmdPal.UI.ViewModels.Models; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Microsoft.CmdPal.UI.ViewModels.UnitTests; + +/// +/// Verifies that the author-specified frozen value from the initialize handshake +/// metadata flows through the proxy (cp-B). When no metadata is supplied the wire +/// default is frozen; when the handshake provides a value it wins. +/// +[TestClass] +public class JSCommandProviderProxyFrozenTests +{ + private static JsonElement Parse(string json) + { + // The returned element must outlive this call, so parse into a document whose + // lifetime is tied to the element via Clone rather than a disposed document. + using var document = JsonDocument.Parse(json); + return document.RootElement.Clone(); + } + + [TestMethod] + public void Frozen_DefaultsToTrue_WhenNoMetadata() + { + using var fake = new JSFakeExtension(); + var provider = new JSCommandProviderProxy(fake.Connection, new JSExtensionManifest { Name = "ext" }); + + Assert.IsTrue(provider.Frozen); + + provider.Dispose(); + } + + [TestMethod] + public void Frozen_HonorsCtorMetadata_False() + { + using var fake = new JSFakeExtension(); + var provider = new JSCommandProviderProxy( + fake.Connection, + new JSExtensionManifest { Name = "ext" }, + Parse("""{ "frozen": false }""")); + + Assert.IsFalse(provider.Frozen); + + provider.Dispose(); + } + + [TestMethod] + public void Frozen_HonorsHandshakeMetadata_False() + { + using var fake = new JSFakeExtension(); + var provider = new JSCommandProviderProxy(fake.Connection, new JSExtensionManifest { Name = "ext" }); + + provider.SetProviderMetadata(Parse("""{ "Frozen": false }""")); + + Assert.IsFalse(provider.Frozen); + + provider.Dispose(); + } + + [TestMethod] + public void Frozen_HonorsHandshakeMetadata_True() + { + using var fake = new JSFakeExtension(); + var provider = new JSCommandProviderProxy(fake.Connection, new JSExtensionManifest { Name = "ext" }); + + provider.SetProviderMetadata(Parse("""{ "frozen": true }""")); + + Assert.IsTrue(provider.Frozen); + + provider.Dispose(); + } +} diff --git a/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JSCommandProviderProxyStartupNotificationTests.cs b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JSCommandProviderProxyStartupNotificationTests.cs new file mode 100644 index 0000000000..d689a55c2f --- /dev/null +++ b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JSCommandProviderProxyStartupNotificationTests.cs @@ -0,0 +1,91 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Text.Json.Nodes; +using System.Threading.Tasks; +using Microsoft.CmdPal.UI.ViewModels.Models; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Microsoft.CmdPal.UI.ViewModels.UnitTests; + +/// +/// Verifies that host notifications an extension emits while it activates, before the +/// host is attached, are buffered and delivered once the host attaches (p4-05). The +/// proxy is constructed first so its notification handlers are registered in time to +/// receive those startup notifications. +/// +[TestClass] +public class JSCommandProviderProxyStartupNotificationTests +{ + private static JSCommandProviderProxy CreateProvider(JSFakeExtension fake) => + new(fake.Connection, new JSExtensionManifest { Name = "startup.ext", DisplayName = "Startup Extension" }); + + [TestMethod] + public async Task StartupStatus_EmittedBeforeHostAttaches_IsDeliveredAfterAttach() + { + using var fake = new JSFakeExtension(); + var provider = CreateProvider(fake); + + // The extension raises a status during activation, before the host is attached. + await fake.PushNotificationAsync( + "host/showStatus", + new JsonObject + { + ["statusId"] = "startup-1", + ["message"] = new JsonObject { ["Message"] = "Starting", ["State"] = 0 }, + }); + + var host = new RecordingExtensionHost(); + provider.InitializeWithHost(host); + + await host.WaitForShownCountAsync(1); + Assert.AreEqual(1, host.Shown.Count); + Assert.AreEqual("Starting", host.Shown[0].Message); + + provider.Dispose(); + } + + [TestMethod] + public async Task StartupLog_EmittedBeforeHostAttaches_IsDeliveredAfterAttach() + { + using var fake = new JSFakeExtension(); + var provider = CreateProvider(fake); + + await fake.PushNotificationAsync( + "host/logMessage", + new JsonObject { ["message"] = "activating", ["state"] = 0 }); + + var host = new RecordingExtensionHost(); + provider.InitializeWithHost(host); + + await host.WaitForLogCountAsync(1); + Assert.AreEqual("activating", host.Logs[0].Message); + + provider.Dispose(); + } + + [TestMethod] + public async Task BufferedActions_AreDeliveredInArrivalOrder() + { + using var fake = new JSFakeExtension(); + var provider = CreateProvider(fake); + + await fake.PushNotificationAsync( + "host/logMessage", + new JsonObject { ["message"] = "first", ["state"] = 0 }); + await fake.PushNotificationAsync( + "host/logMessage", + new JsonObject { ["message"] = "second", ["state"] = 0 }); + + var host = new RecordingExtensionHost(); + provider.InitializeWithHost(host); + + await host.WaitForLogCountAsync(2); + Assert.AreEqual("first", host.Logs[0].Message); + Assert.AreEqual("second", host.Logs[1].Message); + + provider.Dispose(); + } +} diff --git a/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JSCommandProviderProxyStatusDisposeRaceTests.cs b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JSCommandProviderProxyStatusDisposeRaceTests.cs new file mode 100644 index 0000000000..ba32a42f7e --- /dev/null +++ b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JSCommandProviderProxyStatusDisposeRaceTests.cs @@ -0,0 +1,95 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System.Linq; +using System.Text.Json.Nodes; +using System.Threading.Tasks; +using Microsoft.CmdPal.UI.ViewModels.Models; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Microsoft.CmdPal.UI.ViewModels.UnitTests; + +/// +/// Verifies that status hiding cannot race disposal and strand UI (r2-p4-08). A status +/// show is only ever delivered to the host while the proxy is not disposed, and disposal +/// hides every active status under the same lock, so once teardown has hidden statuses no +/// later show can resurrect them. The guarantee under test is the invariant "every status +/// the host was shown is also hidden": teardown can drop a show that had not yet been +/// delivered, but it can never leave a delivered show without a matching hide. +/// +[TestClass] +public class JSCommandProviderProxyStatusDisposeRaceTests +{ + private static JSCommandProviderProxy CreateInitialized(JSFakeExtension fake, out RecordingExtensionHost host) + { + host = new RecordingExtensionHost(); + var provider = new JSCommandProviderProxy( + fake.Connection, + new JSExtensionManifest { Name = "status.race.ext", DisplayName = "Status Race Extension" }); + provider.InitializeWithHost(host); + return provider; + } + + private static JsonObject ShowStatus(string statusId, string message) => new() + { + ["statusId"] = statusId, + ["message"] = new JsonObject { ["Message"] = message, ["State"] = 0 }, + }; + + [TestMethod] + public async Task Dispose_HidesActiveStatuses() + { + using var fake = new JSFakeExtension(); + var provider = CreateInitialized(fake, out var host); + + await fake.PushNotificationAsync("host/showStatus", ShowStatus("s1", "Live")); + await host.WaitForShownCountAsync(1); + + provider.Dispose(); + + // A status that was visible at teardown must be hidden exactly once, with the same + // message instance, so nothing is left stranded in the host UI. + Assert.AreEqual(1, host.Hidden.Count); + Assert.AreSame(host.Shown[0], host.Hidden[0]); + } + + [TestMethod] + public async Task ConcurrentShowAndDispose_NeverStrandsStatus() + { + const int Iterations = 50; + + for (var i = 0; i < Iterations; i++) + { + using var fake = new JSFakeExtension(); + var provider = CreateInitialized(fake, out var host); + + // Push a status and tear the proxy down without waiting for the show to be + // delivered, so the show handler races Dispose. Whichever wins the lock, the + // show is either delivered-then-hidden or its late delivery is dropped: it must + // never be recorded as shown without a matching hide. + var pushTask = fake.PushNotificationAsync("host/showStatus", ShowStatus($"s{i}", "Racing")); + provider.Dispose(); + await pushTask; + + // Give the connection's delivery thread a chance to run the (now no-op) handler + // so the assertion sees the settled state rather than an in-flight one. + await Task.Delay(5); + + var shown = host.Shown; + var hidden = host.Hidden; + + // The invariant: every status the host was actually shown must also have been + // hidden (matched by reference). Before the fix, a show delivered after teardown + // hid statuses would have no matching hide and would strand the UI. + foreach (var message in shown) + { + Assert.IsTrue( + hidden.Any(h => ReferenceEquals(h, message)), + "A status shown to the host must always be hidden; disposal must not strand it."); + } + + provider.Dispose(); + } + } +} diff --git a/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JSCommandProviderProxyStatusLifecycleTests.cs b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JSCommandProviderProxyStatusLifecycleTests.cs new file mode 100644 index 0000000000..2ce9d65b1e --- /dev/null +++ b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JSCommandProviderProxyStatusLifecycleTests.cs @@ -0,0 +1,122 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Text.Json.Nodes; +using System.Threading.Tasks; +using Microsoft.CmdPal.UI.ViewModels.Models; +using Microsoft.CommandPalette.Extensions; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Microsoft.CmdPal.UI.ViewModels.UnitTests; + +/// +/// Verifies host status lifecycle handling on the proxy (p4-06): a repeated status id +/// updates the existing message in place rather than stacking a duplicate, a status is +/// removed by id independent of its severity, and all active statuses are cleared when +/// the extension disconnects even without an explicit hide. +/// +[TestClass] +public class JSCommandProviderProxyStatusLifecycleTests +{ + private static JSCommandProviderProxy CreateInitialized(JSFakeExtension fake, out RecordingExtensionHost host) + { + host = new RecordingExtensionHost(); + var provider = new JSCommandProviderProxy( + fake.Connection, + new JSExtensionManifest { Name = "status.ext", DisplayName = "Status Extension" }); + provider.InitializeWithHost(host); + return provider; + } + + private static JsonObject ShowStatus(string statusId, string message, int state) => new() + { + ["statusId"] = statusId, + ["message"] = new JsonObject { ["Message"] = message, ["State"] = state }, + }; + + [TestMethod] + public async Task RepeatedStatusId_UpdatesInPlace_WithoutDuplicateShow() + { + using var fake = new JSFakeExtension(); + var provider = CreateInitialized(fake, out var host); + + await fake.PushNotificationAsync("host/showStatus", ShowStatus("s1", "First", 0)); + await host.WaitForShownCountAsync(1); + + // Update the same status, then push a second distinct status as an ordering + // barrier: once the second status is shown, the in-place update has been applied. + await fake.PushNotificationAsync("host/showStatus", ShowStatus("s1", "Second", 0)); + await fake.PushNotificationAsync("host/showStatus", ShowStatus("s2", "Other", 0)); + await host.WaitForShownCountAsync(2); + + Assert.AreEqual(2, host.Shown.Count, "The repeated status id must not produce a second ShowStatus."); + Assert.AreEqual("Second", host.Shown[0].Message, "The existing status must be updated in place."); + + provider.Dispose(); + } + + [TestMethod] + public async Task Status_RemovedById_IndependentOfSeverity() + { + using var fake = new JSFakeExtension(); + var provider = CreateInitialized(fake, out var host); + + await fake.PushNotificationAsync("host/showStatus", ShowStatus("w1", "Careful", 2)); + await host.WaitForShownCountAsync(1); + Assert.AreEqual(MessageState.Warning, host.Shown[0].State); + + await fake.PushNotificationAsync("host/hideStatus", new JsonObject { ["statusId"] = "w1" }); + await host.WaitForHiddenCountAsync(1); + + Assert.AreEqual(1, host.Hidden.Count); + Assert.AreSame(host.Shown[0], host.Hidden[0], "The hidden status must be the warning shown by that id."); + + provider.Dispose(); + } + + [TestMethod] + public async Task ActiveStatuses_AreCleared_OnDisconnect() + { + using var fake = new JSFakeExtension(); + var provider = CreateInitialized(fake, out var host); + + await fake.PushNotificationAsync("host/showStatus", ShowStatus("s1", "Live", 0)); + await host.WaitForShownCountAsync(1); + + // Disconnecting the extension (its process exiting) must clear active statuses + // even though no explicit hide arrived. + fake.Dispose(); + + await host.WaitForHiddenCountAsync(1); + Assert.AreSame(host.Shown[0], host.Hidden[0]); + + provider.Dispose(); + } + + // r3-p4-06: a hide must never be observed by the host before the show it cancels. The + // handlers dispatch to the host inside the same lock acquisition that mutates the status + // map, so a hide cannot overtake a pending show. Caveat: the underlying defect is a + // cross-thread timing race (the show arrives on the notification worker while a hide can + // arrive from disconnect or dispose); this is a best-effort deterministic guard that + // asserts the host observes show-before-hide and ends with the status cleared. + [TestMethod] + public async Task ShowThenHide_SameId_HostObservesShowBeforeHide() + { + using var fake = new JSFakeExtension(); + var provider = CreateInitialized(fake, out var host); + + await fake.PushNotificationAsync("host/showStatus", ShowStatus("s1", "Live", 0)); + await fake.PushNotificationAsync("host/hideStatus", new JsonObject { ["statusId"] = "s1" }); + + await host.WaitForShownCountAsync(1); + await host.WaitForHiddenCountAsync(1); + + Assert.AreEqual(1, host.Shown.Count); + Assert.AreEqual(1, host.Hidden.Count); + Assert.AreSame(host.Shown[0], host.Hidden[0], "The hidden status must be the one that was shown."); + + provider.Dispose(); + } +} diff --git a/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JSExtensionWrapperBootstrapTests.cs b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JSExtensionWrapperBootstrapTests.cs new file mode 100644 index 0000000000..634b335c86 --- /dev/null +++ b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JSExtensionWrapperBootstrapTests.cs @@ -0,0 +1,126 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System.IO; +using Microsoft.CmdPal.UI.ViewModels.Models; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Microsoft.CmdPal.UI.ViewModels.UnitTests; + +/// +/// Verifies that the host launch goes through the Phase 1 SDK bootstrap (r3-p4-07). The +/// bootstrap claims and guards stdout before it dynamically imports the extension entry, so +/// static top-level stdout writes cannot corrupt the JSON-RPC framing. The launcher must +/// resolve the bootstrap relative to the extension's installed SDK under +/// node_modules/@microsoft/cmdpal-sdk, preferring the package's declared +/// bin entry, and must return null (falling back to a direct entry launch) when the +/// SDK or its bootstrap is absent. +/// +[TestClass] +public class JSExtensionWrapperBootstrapTests +{ + private static string CreateSdkRoot(string manifestDir) + { + var sdkRoot = Path.Combine(manifestDir, "node_modules", "@microsoft", "cmdpal-sdk"); + Directory.CreateDirectory(sdkRoot); + return sdkRoot; + } + + private static string NewTempDir() + { + var dir = Path.Combine(Path.GetTempPath(), "cmdpal-bootstrap-test-" + Path.GetRandomFileName()); + Directory.CreateDirectory(dir); + return dir; + } + + [TestMethod] + public void ResolveBootstrapScript_PrefersPackageJsonBinEntry() + { + var manifestDir = NewTempDir(); + try + { + var sdkRoot = CreateSdkRoot(manifestDir); + var binTarget = Path.Combine(sdkRoot, "dist", "runtime", "bootstrap.js"); + Directory.CreateDirectory(Path.GetDirectoryName(binTarget)!); + File.WriteAllText(binTarget, "// bootstrap"); + File.WriteAllText( + Path.Combine(sdkRoot, "package.json"), + "{ \"name\": \"@microsoft/cmdpal-sdk\", \"bin\": { \"cmdpal-bootstrap\": \"./dist/runtime/bootstrap.js\" } }"); + + var resolved = JSExtensionWrapper.ResolveBootstrapScript(manifestDir); + + Assert.IsNotNull(resolved); + Assert.AreEqual(Path.GetFullPath(binTarget), Path.GetFullPath(resolved!)); + } + finally + { + Directory.Delete(manifestDir, recursive: true); + } + } + + [TestMethod] + public void ResolveBootstrapScript_FallsBackToKnownArtifact_WhenNoBinEntry() + { + var manifestDir = NewTempDir(); + try + { + var sdkRoot = CreateSdkRoot(manifestDir); + File.WriteAllText(Path.Combine(sdkRoot, "package.json"), "{ \"name\": \"@microsoft/cmdpal-sdk\" }"); + + var fallback = Path.Combine(sdkRoot, "dist", "runtime", "bootstrap.js"); + Directory.CreateDirectory(Path.GetDirectoryName(fallback)!); + File.WriteAllText(fallback, "// bootstrap"); + + var resolved = JSExtensionWrapper.ResolveBootstrapScript(manifestDir); + + Assert.IsNotNull(resolved); + Assert.AreEqual(Path.GetFullPath(fallback), Path.GetFullPath(resolved!)); + } + finally + { + Directory.Delete(manifestDir, recursive: true); + } + } + + [TestMethod] + public void ResolveBootstrapScript_ReturnsNull_WhenSdkMissing() + { + var manifestDir = NewTempDir(); + try + { + Assert.IsNull(JSExtensionWrapper.ResolveBootstrapScript(manifestDir)); + } + finally + { + Directory.Delete(manifestDir, recursive: true); + } + } + + [TestMethod] + public void ResolveBootstrapScript_ReturnsNull_WhenBootstrapArtifactMissing() + { + var manifestDir = NewTempDir(); + try + { + // The SDK is installed but neither the bin entry target nor the known artifacts + // exist on disk, so no bootstrap can be launched. + var sdkRoot = CreateSdkRoot(manifestDir); + File.WriteAllText( + Path.Combine(sdkRoot, "package.json"), + "{ \"name\": \"@microsoft/cmdpal-sdk\", \"bin\": { \"cmdpal-bootstrap\": \"./dist/runtime/bootstrap.js\" } }"); + + Assert.IsNull(JSExtensionWrapper.ResolveBootstrapScript(manifestDir)); + } + finally + { + Directory.Delete(manifestDir, recursive: true); + } + } + + [TestMethod] + public void ResolveBootstrapScript_ReturnsNull_ForEmptyManifestDirectory() + { + Assert.IsNull(JSExtensionWrapper.ResolveBootstrapScript(string.Empty)); + } +} diff --git a/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JSExtensionWrapperTests.cs b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JSExtensionWrapperTests.cs new file mode 100644 index 0000000000..9050d77212 --- /dev/null +++ b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JSExtensionWrapperTests.cs @@ -0,0 +1,123 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.IO; +using Microsoft.CmdPal.UI.ViewModels.Models; +using Microsoft.CommandPalette.Extensions; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Microsoft.CmdPal.UI.ViewModels.UnitTests; + +[TestClass] +public class JSExtensionWrapperTests +{ + private static JSExtensionWrapper CreateWrapper(string? version = "1.2.3") => new( + new JSExtensionManifest + { + Name = "test-ext", + DisplayName = "Test Extension", + Version = version, + Publisher = "unit-test", + Main = "index.js", + EntryPointPath = Path.Combine(Path.GetTempPath(), "index.js"), + }, + Path.Combine(Path.GetTempPath(), "test-ext")); + + [TestMethod] + public void NewWrapper_IsHealthy_WithZeroCrashes() + { + var wrapper = CreateWrapper(); + Assert.IsTrue(wrapper.IsHealthy); + Assert.AreEqual(0, wrapper.ConsecutiveCrashCount); + Assert.IsFalse(wrapper.IsRunning()); + } + + [TestMethod] + public void RecordUnexpectedExit_WithinThreshold_StaysHealthy() + { + var wrapper = CreateWrapper(); + + for (var i = 1; i <= 3; i++) + { + Assert.AreEqual(i, wrapper.RecordUnexpectedExit()); + } + + Assert.IsTrue(wrapper.IsHealthy, "Three or fewer crashes should remain healthy."); + Assert.AreEqual(3, wrapper.ConsecutiveCrashCount); + } + + [TestMethod] + public void RecordUnexpectedExit_AboveThreshold_BecomesUnhealthy() + { + var wrapper = CreateWrapper(); + + for (var i = 0; i < 4; i++) + { + wrapper.RecordUnexpectedExit(); + } + + Assert.IsFalse(wrapper.IsHealthy, "More than three crashes should mark the extension unhealthy."); + Assert.AreEqual(4, wrapper.ConsecutiveCrashCount); + } + + [TestMethod] + public void ResetCrashCount_RestoresHealthAndCount() + { + var wrapper = CreateWrapper(); + + for (var i = 0; i < 5; i++) + { + wrapper.RecordUnexpectedExit(); + } + + Assert.IsFalse(wrapper.IsHealthy); + + wrapper.ResetCrashCount(); + + Assert.IsTrue(wrapper.IsHealthy); + Assert.AreEqual(0, wrapper.ConsecutiveCrashCount); + } + + [TestMethod] + public void Identity_DerivesFromManifest() + { + var wrapper = CreateWrapper(); + + Assert.AreEqual("js!test-ext", wrapper.ExtensionUniqueId); + Assert.AreEqual("js!test-ext", wrapper.PackageFamilyName); + Assert.AreEqual("Test Extension", wrapper.ExtensionDisplayName); + Assert.AreEqual("unit-test", wrapper.Publisher); + Assert.IsTrue(wrapper.HasProviderType(ProviderType.Commands)); + } + + [TestMethod] + public void Version_ParsesManifestVersion() + { + var wrapper = CreateWrapper("4.5.6"); + var version = wrapper.Version; + + Assert.AreEqual(4, version.Major); + Assert.AreEqual(5, version.Minor); + Assert.AreEqual(6, version.Build); + } + + [TestMethod] + public void Version_MissingVersion_DefaultsToOneZeroZero() + { + var wrapper = CreateWrapper(version: null); + var version = wrapper.Version; + + Assert.AreEqual(1, version.Major); + Assert.AreEqual(0, version.Minor); + Assert.AreEqual(0, version.Build); + } + + [TestMethod] + public void GetExtensionObject_IsNull_ForJsExtensions() + { + var wrapper = CreateWrapper(); + Assert.IsNull(wrapper.GetExtensionObject()); + } +} diff --git a/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JSListItemAdapterKeyTests.cs b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JSListItemAdapterKeyTests.cs new file mode 100644 index 0000000000..873f778a2d --- /dev/null +++ b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JSListItemAdapterKeyTests.cs @@ -0,0 +1,62 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System.Text.Json; +using Microsoft.CmdPal.UI.ViewModels.Models; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Microsoft.CmdPal.UI.ViewModels.UnitTests; + +/// +/// Verifies the stable list-item identity used to reuse item adapters across a +/// refresh (p4-03). Keying by the SDK-emitted stable id (or nested command id) rather +/// than the title keeps each row bound to its own command when a refresh reorders +/// items that share a title, so a duplicate-title reorder does not swap actions. +/// +[TestClass] +public class JSListItemAdapterKeyTests +{ + private static string Key(string json) + { + using var doc = JsonDocument.Parse(json); + return JSListItemAdapter.ComputeKey(doc.RootElement); + } + + [TestMethod] + public void ComputeKey_PrefersStableId() + { + Assert.AreEqual("id:item-42", Key("""{ "id": "item-42", "title": "Anything" }""")); + } + + [TestMethod] + public void ComputeKey_FallsBackToCommandId() + { + Assert.AreEqual("cmd:cmd-7", Key("""{ "title": "Anything", "command": { "id": "cmd-7" } }""")); + } + + [TestMethod] + public void ComputeKey_FallsBackToTitleWhenNoId() + { + Assert.AreEqual("title:Only Title", Key("""{ "title": "Only Title" }""")); + } + + [TestMethod] + public void ComputeKey_IdIsNamespacedApartFromTitle() + { + // An id "X" must not produce the same key as a title "X". + var idKey = Key("""{ "id": "X" }"""); + var titleKey = Key("""{ "title": "X" }"""); + Assert.AreNotEqual(idKey, titleKey); + } + + [TestMethod] + public void ComputeKey_DuplicateTitlesWithDistinctIds_ProduceDistinctKeys() + { + // The duplicate-title reorder scenario: two rows share a title but have their + // own ids, so they get distinct keys and stay bound to their own commands. + var first = Key("""{ "id": "a", "title": "Same Title" }"""); + var second = Key("""{ "id": "b", "title": "Same Title" }"""); + Assert.AreNotEqual(first, second); + } +} diff --git a/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JsonRpcExtensionServiceCrashRecoveryTests.cs b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JsonRpcExtensionServiceCrashRecoveryTests.cs new file mode 100644 index 0000000000..4521926eb5 --- /dev/null +++ b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JsonRpcExtensionServiceCrashRecoveryTests.cs @@ -0,0 +1,68 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using Microsoft.CmdPal.UI.ViewModels.Services; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Microsoft.CmdPal.UI.ViewModels.UnitTests; + +/// +/// Documents the crash-lifecycle decisions behind r2-p4-03 (an immediate post-init +/// crash must be handled) and r2-p4-07 (a crash-disabled extension recovers after a +/// source edit). The end-to-end wiring (the post-init IsRunning() probe that +/// drives OnExtensionProcessExited, the disable branch keeping the source +/// watcher alive, and hot-reload resetting the crash count) requires spawning a Node +/// process and is verified by inspection; the deterministic decision the wiring relies +/// on is exercised here through the pure +/// seam. +/// +[TestClass] +public class JsonRpcExtensionServiceCrashRecoveryTests +{ + private const int MaxRestartAttempts = 3; + + [TestMethod] + public void CrashSequence_ReachesDisableAfterExceedingLimit() + { + // Each recorded crash increments the count; the service restarts while at or below + // the limit and disables only once the count exceeds it. A crash observed + // immediately after init (p4-03) feeds this same counter, so an extension that + // exits right after starting is not treated as healthy. + var crashCount = 0; + for (var attempt = 1; attempt <= MaxRestartAttempts; attempt++) + { + crashCount++; + Assert.AreEqual( + JsonRpcExtensionService.CrashAction.Restart, + JsonRpcExtensionService.DecideCrashAction(crashCount, MaxRestartAttempts), + $"Crash {crashCount} is within the limit and must restart."); + } + + crashCount++; + Assert.AreEqual( + JsonRpcExtensionService.CrashAction.Disable, + JsonRpcExtensionService.DecideCrashAction(crashCount, MaxRestartAttempts), + "Exceeding the restart limit must disable the extension."); + } + + [TestMethod] + public void SourceEdit_ResetsCrashCount_AllowsRestartAgain() + { + // Drive the extension to the disabled decision. + var crashCount = MaxRestartAttempts + 1; + Assert.AreEqual( + JsonRpcExtensionService.CrashAction.Disable, + JsonRpcExtensionService.DecideCrashAction(crashCount, MaxRestartAttempts)); + + // A source edit hot-reloads with resetCrashCount: true, which clears the counter + // for the directory. The very next crash decision must be Restart again, so the + // extension is no longer stranded in the disabled state (p4-07). + crashCount = 0; + crashCount++; + Assert.AreEqual( + JsonRpcExtensionService.CrashAction.Restart, + JsonRpcExtensionService.DecideCrashAction(crashCount, MaxRestartAttempts), + "After a source edit resets the crash count, the extension must retry loading."); + } +} diff --git a/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JsonRpcExtensionServiceDiscoveryTests.cs b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JsonRpcExtensionServiceDiscoveryTests.cs new file mode 100644 index 0000000000..89248a523a --- /dev/null +++ b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JsonRpcExtensionServiceDiscoveryTests.cs @@ -0,0 +1,149 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.IO; +using Microsoft.CmdPal.UI.ViewModels.Services; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Microsoft.CmdPal.UI.ViewModels.UnitTests; + +[TestClass] +public class JsonRpcExtensionServiceDiscoveryTests +{ + private string _root = null!; + + [TestInitialize] + public void Setup() + { + _root = Path.Combine(Path.GetTempPath(), $"JSExtDiscovery_{Guid.NewGuid():N}"); + Directory.CreateDirectory(_root); + } + + [TestCleanup] + public void Cleanup() + { + if (Directory.Exists(_root)) + { + Directory.Delete(_root, recursive: true); + } + } + + [TestMethod] + public void DiscoverManifests_ReturnsOnlyValidCmdPalExtensions() + { + // Valid: has cmdpal section + resolvable entry point. + const string GoodJson = """ + { + "name": "good-ext", + "main": "index.js", + "cmdpal": { "displayName": "Good" } + } + """; + CreateExtension("good", GoodJson, "index.js"); + + // Invalid: no cmdpal section. + const string NoCmdPalJson = """ + { + "name": "plain-ext", + "main": "index.js" + } + """; + CreateExtension("no-cmdpal", NoCmdPalJson, "index.js"); + + // Invalid: cmdpal section but entry point does not exist. + const string MissingEntryJson = """ + { + "name": "missing-entry-ext", + "main": "index.js", + "cmdpal": {} + } + """; + CreateExtension("missing-entry", MissingEntryJson, entryPointRelativePath: null); + + // Invalid: directory without a package.json at all. + Directory.CreateDirectory(Path.Combine(_root, "empty")); + + var results = JsonRpcExtensionService.DiscoverManifests(_root); + + Assert.AreEqual(1, results.Count); + Assert.AreEqual("good-ext", results[0].Manifest.Name); + Assert.AreEqual(Path.Combine(_root, "good"), results[0].Directory); + } + + [TestMethod] + public void DiscoverManifests_MissingRoot_ReturnsEmpty() + { + var results = JsonRpcExtensionService.DiscoverManifests(Path.Combine(_root, "does-not-exist")); + Assert.AreEqual(0, results.Count); + } + + [TestMethod] + public void DiscoverManifests_EmptyRoot_ReturnsEmpty() + { + var results = JsonRpcExtensionService.DiscoverManifests(_root); + Assert.AreEqual(0, results.Count); + } + + [TestMethod] + public void DecideCrashAction_AtOrBelowLimit_Restarts() + { + Assert.AreEqual(JsonRpcExtensionService.CrashAction.Restart, JsonRpcExtensionService.DecideCrashAction(1, 3)); + Assert.AreEqual(JsonRpcExtensionService.CrashAction.Restart, JsonRpcExtensionService.DecideCrashAction(2, 3)); + Assert.AreEqual(JsonRpcExtensionService.CrashAction.Restart, JsonRpcExtensionService.DecideCrashAction(3, 3)); + } + + [TestMethod] + public void DecideCrashAction_AboveLimit_Disables() + { + Assert.AreEqual(JsonRpcExtensionService.CrashAction.Disable, JsonRpcExtensionService.DecideCrashAction(4, 3)); + Assert.AreEqual(JsonRpcExtensionService.CrashAction.Disable, JsonRpcExtensionService.DecideCrashAction(10, 3)); + } + + [TestMethod] + public void IsUnderDirectory_SamePath_IsTrue() + { + var dir = Path.Combine(_root, "foo"); + Assert.IsTrue(JsonRpcExtensionService.IsUnderDirectory(dir, dir)); + Assert.IsTrue(JsonRpcExtensionService.IsUnderDirectory(dir + Path.DirectorySeparatorChar, dir)); + } + + [TestMethod] + public void IsUnderDirectory_Descendant_IsTrue() + { + var dir = Path.Combine(_root, "foo"); + var file = Path.Combine(dir, "src", "index.js"); + Assert.IsTrue(JsonRpcExtensionService.IsUnderDirectory(file, dir)); + } + + [TestMethod] + public void IsUnderDirectory_SiblingWithSharedPrefix_IsFalse() + { + // "foo-bar" must not be considered a child of "foo". + var dir = Path.Combine(_root, "foo"); + var sibling = Path.Combine(_root, "foo-bar", "index.js"); + Assert.IsFalse(JsonRpcExtensionService.IsUnderDirectory(sibling, dir)); + } + + [TestMethod] + public void IsUnderDirectory_EmptyArguments_IsFalse() + { + Assert.IsFalse(JsonRpcExtensionService.IsUnderDirectory(string.Empty, _root)); + Assert.IsFalse(JsonRpcExtensionService.IsUnderDirectory(_root, string.Empty)); + } + + private void CreateExtension(string dirName, string packageJson, string? entryPointRelativePath) + { + var dir = Path.Combine(_root, dirName); + Directory.CreateDirectory(dir); + File.WriteAllText(Path.Combine(dir, "package.json"), packageJson); + + if (entryPointRelativePath is not null) + { + var entryPath = Path.Combine(dir, entryPointRelativePath); + Directory.CreateDirectory(Path.GetDirectoryName(entryPath)!); + File.WriteAllText(entryPath, "// entry"); + } + } +} diff --git a/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JsonRpcExtensionServiceReconciliationTests.cs b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JsonRpcExtensionServiceReconciliationTests.cs new file mode 100644 index 0000000000..35d325ef73 --- /dev/null +++ b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JsonRpcExtensionServiceReconciliationTests.cs @@ -0,0 +1,202 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CmdPal.UI.ViewModels.Models; +using Microsoft.CmdPal.UI.ViewModels.Services; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Microsoft.CmdPal.UI.ViewModels.UnitTests; + +/// +/// Covers the robust-discovery and duplicate-id remediations (p4-04, p4-07): the +/// reconciliation diff, the deterministic collision policy, the manifest-stability +/// retry, and mapping a changed path back to its owning extension directory. +/// +[TestClass] +public class JsonRpcExtensionServiceReconciliationTests +{ + private string _root = null!; + + [TestInitialize] + public void Setup() + { + _root = Path.Combine(Path.GetTempPath(), $"JSExtReconcile_{Guid.NewGuid():N}"); + Directory.CreateDirectory(_root); + } + + [TestCleanup] + public void Cleanup() + { + if (Directory.Exists(_root)) + { + Directory.Delete(_root, recursive: true); + } + } + + [TestMethod] + public void ReconcileDirectories_ComputesAddsAndRemoves() + { + var discovered = new[] + { + @"C:\ext\alpha", + @"C:\ext\beta", + @"C:\ext\gamma", + }; + + var loaded = new[] + { + @"C:\ext\beta\", // trailing separator, still the same directory + @"C:\ext\DELTA", // loaded but no longer on disk + }; + + var (toAdd, toRemove) = JsonRpcExtensionService.ReconcileDirectories(discovered, loaded); + + var addSet = toAdd.Select(DirectoryLifecycleGate.Canonicalize).ToHashSet(StringComparer.OrdinalIgnoreCase); + var removeSet = toRemove.Select(DirectoryLifecycleGate.Canonicalize).ToHashSet(StringComparer.OrdinalIgnoreCase); + + Assert.IsTrue(addSet.Contains(DirectoryLifecycleGate.Canonicalize(@"C:\ext\alpha"))); + Assert.IsTrue(addSet.Contains(DirectoryLifecycleGate.Canonicalize(@"C:\ext\gamma"))); + Assert.IsFalse(addSet.Contains(DirectoryLifecycleGate.Canonicalize(@"C:\ext\beta")), "Already-loaded beta is not re-added."); + Assert.AreEqual(2, addSet.Count); + + Assert.IsTrue(removeSet.Contains(DirectoryLifecycleGate.Canonicalize(@"C:\ext\DELTA")), "A loaded-but-deleted extension is reconciled out."); + Assert.AreEqual(1, removeSet.Count); + } + + [TestMethod] + public void ResolveIdCollisions_DuplicateIds_DeterministicWinnerByPath() + { + // Two extensions in different directories advertise the same name key. + CreateExtension("z-dir", "dup-ext"); + CreateExtension("a-dir", "dup-ext"); + CreateExtension("solo", "unique-ext"); + + var discovered = JsonRpcExtensionService.DiscoverManifests(_root); + var (accepted, rejected) = JsonRpcExtensionService.ResolveIdCollisions(discovered); + + // The winner is deterministic: the canonical-path-sorted first directory wins, + // independent of enumeration order. "a-dir" sorts before "z-dir". + var acceptedDirs = accepted.Select(a => Path.GetFileName(a.Directory)).ToHashSet(StringComparer.OrdinalIgnoreCase); + Assert.IsTrue(acceptedDirs.Contains("a-dir"), "The path-sorted first duplicate wins."); + Assert.IsTrue(acceptedDirs.Contains("solo"), "A non-duplicate is always accepted."); + Assert.IsFalse(acceptedDirs.Contains("z-dir"), "The losing duplicate is rejected."); + + Assert.AreEqual(1, rejected.Count); + Assert.AreEqual("z-dir", Path.GetFileName(rejected[0].Directory)); + Assert.AreEqual( + DirectoryLifecycleGate.Canonicalize(Path.Combine(_root, "a-dir")), + rejected[0].WinnerDirectory, + "The rejection records the deterministic winner directory."); + } + + [TestMethod] + public void ResolveIdCollisions_IsStableAcrossInputOrder() + { + CreateExtension("z-dir", "dup-ext"); + CreateExtension("a-dir", "dup-ext"); + + var discovered = JsonRpcExtensionService.DiscoverManifests(_root).ToList(); + + var forward = JsonRpcExtensionService.ResolveIdCollisions(discovered); + discovered.Reverse(); + var reversed = JsonRpcExtensionService.ResolveIdCollisions(discovered); + + var forwardWinner = Path.GetFileName(forward.Accepted.Single().Directory); + var reversedWinner = Path.GetFileName(reversed.Accepted.Single().Directory); + + Assert.AreEqual("a-dir", forwardWinner); + Assert.AreEqual(forwardWinner, reversedWinner, "The winner is independent of input order."); + } + + [TestMethod] + public async Task WaitForStableManifestAsync_RetriesUntilManifestParses() + { + var attempts = 0; + JSExtensionManifestParseResult Parse(string manifestPath) + { + attempts++; + + // Simulate a slow install: the first two reads see a partially written + // package that does not parse, the third read succeeds. + if (attempts < 3) + { + return JSExtensionManifestParseResult.Failure("still being written"); + } + + return JSExtensionManifestParseResult.Success(new JSExtensionManifest { Name = "slow-ext" }); + } + + var delays = 0; + Task Delay(int attempt, CancellationToken token) + { + delays++; + return Task.CompletedTask; + } + + var manifest = await JsonRpcExtensionService.WaitForStableManifestAsync( + "package.json", + attempts: 5, + Parse, + Delay, + CancellationToken.None); + + Assert.IsNotNull(manifest); + Assert.AreEqual("slow-ext", manifest!.Name); + Assert.AreEqual(3, attempts); + Assert.AreEqual(2, delays, "It waited between the failed attempts."); + } + + [TestMethod] + public async Task WaitForStableManifestAsync_NeverValid_ReturnsNull() + { + var manifest = await JsonRpcExtensionService.WaitForStableManifestAsync( + "package.json", + attempts: 3, + _ => JSExtensionManifestParseResult.Failure("bad"), + (_, _) => Task.CompletedTask, + CancellationToken.None); + + Assert.IsNull(manifest); + } + + [TestMethod] + public void GetExtensionDirectoryForPath_ReturnsOwningTopLevelDirectory() + { + var manifestPath = Path.Combine(_root, "my-ext", "package.json"); + var sourcePath = Path.Combine(_root, "my-ext", "src", "index.js"); + + var expected = Path.Combine(Path.GetFullPath(_root), "my-ext"); + + Assert.AreEqual(expected, JsonRpcExtensionService.GetExtensionDirectoryForPath(_root, manifestPath)); + Assert.AreEqual(expected, JsonRpcExtensionService.GetExtensionDirectoryForPath(_root, sourcePath)); + } + + [TestMethod] + public void GetExtensionDirectoryForPath_PathOutsideRoot_ReturnsNull() + { + Assert.IsNull(JsonRpcExtensionService.GetExtensionDirectoryForPath(_root, @"C:\somewhere\else\file.js")); + Assert.IsNull(JsonRpcExtensionService.GetExtensionDirectoryForPath(_root, _root)); + } + + private void CreateExtension(string dirName, string extensionName) + { + var dir = Path.Combine(_root, dirName); + Directory.CreateDirectory(dir); + var packageJson = $$""" + { + "name": "{{extensionName}}", + "main": "index.js", + "cmdpal": { "displayName": "{{extensionName}}" } + } + """; + File.WriteAllText(Path.Combine(dir, "package.json"), packageJson); + File.WriteAllText(Path.Combine(dir, "index.js"), "// entry"); + } +} diff --git a/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JsonRpcExtensionServiceWatcherRoutingTests.cs b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JsonRpcExtensionServiceWatcherRoutingTests.cs new file mode 100644 index 0000000000..aab37f51b3 --- /dev/null +++ b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/JsonRpcExtensionServiceWatcherRoutingTests.cs @@ -0,0 +1,235 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System.IO; +using Microsoft.CmdPal.UI.ViewModels.Models; +using Microsoft.CmdPal.UI.ViewModels.Services; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Microsoft.CmdPal.UI.ViewModels.UnitTests; + +/// +/// Verifies the pure watcher-routing decisions used by the directory and source +/// watchers (r2-p4-04, r2-p4-05, r2-p4-06). These are the same decisions the live +/// watchers make, extracted so they can be tested without spinning up a real +/// FileSystemWatcher or a Node process: +/// +/// Churn under node_modules/.git subtrees must not trigger reloads (p4-04). +/// Rename/delete-derived source paths still route to a reload (p4-05). +/// A manifest edit is detectable so an explicit refresh reloads it (p4-06). +/// +/// +[TestClass] +public class JsonRpcExtensionServiceWatcherRoutingTests +{ + private static string Path3(string a, string b, string c) => Path.Combine(a, b, c); + + [TestMethod] + public void HasIgnoredDirectorySegment_NodeModules_IsTrue() + { + var path = Path.Combine(@"C:\ext\my-extension", "node_modules", "left-pad", "index.js"); + Assert.IsTrue(JsonRpcExtensionService.HasIgnoredDirectorySegment(path)); + } + + [TestMethod] + public void HasIgnoredDirectorySegment_NestedNodeModules_IsTrue() + { + // A deeply nested node_modules tree (the restart-storm source) must still be caught. + var path = Path.Combine( + @"C:\ext\my-extension", + "node_modules", + "a", + "node_modules", + "b", + "package.json"); + Assert.IsTrue(JsonRpcExtensionService.HasIgnoredDirectorySegment(path)); + } + + [TestMethod] + public void HasIgnoredDirectorySegment_GitFolder_IsTrue() + { + var path = Path3(@"C:\ext\my-extension", ".git", "index"); + Assert.IsTrue(JsonRpcExtensionService.HasIgnoredDirectorySegment(path)); + } + + [TestMethod] + public void HasIgnoredDirectorySegment_SimilarlyNamedFolder_IsFalse() + { + // A directory whose name merely contains "node_modules" is not the real thing. + var path = Path3(@"C:\ext\my-extension", "node_modules_backup", "index.js"); + Assert.IsFalse(JsonRpcExtensionService.HasIgnoredDirectorySegment(path)); + + var git = Path3(@"C:\ext\my-extension", "gitignore-samples", "index.js"); + Assert.IsFalse(JsonRpcExtensionService.HasIgnoredDirectorySegment(git)); + } + + [TestMethod] + public void HasIgnoredDirectorySegment_ForwardSlashes_AreHonored() + { + Assert.IsTrue(JsonRpcExtensionService.HasIgnoredDirectorySegment("C:/ext/my-extension/node_modules/pkg/index.js")); + } + + [TestMethod] + public void HasIgnoredDirectorySegment_Empty_IsFalse() + { + Assert.IsFalse(JsonRpcExtensionService.HasIgnoredDirectorySegment(string.Empty)); + Assert.IsFalse(JsonRpcExtensionService.HasIgnoredDirectorySegment(null!)); + } + + [TestMethod] + public void ShouldReloadForSourceChange_JavaScriptSource_IsTrue() + { + // A plain source edit, a rename target, and a delete all arrive as full paths; + // each must route to a reload. + Assert.IsTrue(JsonRpcExtensionService.ShouldReloadForSourceChange(Path3(@"C:\ext\my-extension", "src", "index.js"))); + Assert.IsTrue(JsonRpcExtensionService.ShouldReloadForSourceChange(Path.Combine(@"C:\ext\my-extension", "commands.mjs"))); + Assert.IsTrue(JsonRpcExtensionService.ShouldReloadForSourceChange(Path.Combine(@"C:\ext\my-extension", "legacy.cjs"))); + } + + [TestMethod] + public void ShouldReloadForSourceChange_UnderNodeModules_IsFalse() + { + // Even though it is a .js file, a change under node_modules must never reload. + var path = Path.Combine(@"C:\ext\my-extension", "node_modules", "dep", "index.js"); + Assert.IsFalse(JsonRpcExtensionService.ShouldReloadForSourceChange(path)); + } + + [TestMethod] + public void ShouldReloadForSourceChange_NonSourceFile_IsFalse() + { + Assert.IsFalse(JsonRpcExtensionService.ShouldReloadForSourceChange(Path.Combine(@"C:\ext\my-extension", "README.md"))); + Assert.IsFalse(JsonRpcExtensionService.ShouldReloadForSourceChange(Path.Combine(@"C:\ext\my-extension", "styles.css"))); + } + + [TestMethod] + public void ShouldReloadForSourceChange_Empty_IsFalse() + { + Assert.IsFalse(JsonRpcExtensionService.ShouldReloadForSourceChange(string.Empty)); + Assert.IsFalse(JsonRpcExtensionService.ShouldReloadForSourceChange(null!)); + } + + [TestMethod] + public void ManifestChanged_IdenticalManifests_IsFalse() + { + var manifest = SampleManifest(); + Assert.IsFalse(JsonRpcExtensionService.ManifestChanged(manifest, manifest with { })); + } + + [TestMethod] + public void ManifestChanged_DisplayNameEdited_IsTrue() + { + var loaded = SampleManifest(); + var current = loaded with { DisplayName = "Renamed Extension" }; + Assert.IsTrue(JsonRpcExtensionService.ManifestChanged(loaded, current)); + } + + [TestMethod] + public void ManifestChanged_VersionEdited_IsTrue() + { + var loaded = SampleManifest(); + Assert.IsTrue(JsonRpcExtensionService.ManifestChanged(loaded, loaded with { Version = "2.0.0" })); + } + + [TestMethod] + public void ManifestChanged_EntryPointEdited_IsTrue() + { + var loaded = SampleManifest(); + var current = loaded with { EntryPointPath = @"C:\ext\my-extension\dist\index.js" }; + Assert.IsTrue(JsonRpcExtensionService.ManifestChanged(loaded, current)); + } + + [TestMethod] + public void ManifestChanged_DebugToggled_IsTrue() + { + var loaded = SampleManifest(); + Assert.IsTrue(JsonRpcExtensionService.ManifestChanged(loaded, loaded with { Debug = true })); + Assert.IsTrue(JsonRpcExtensionService.ManifestChanged(loaded, loaded with { DebugPort = 9333 })); + } + + [TestMethod] + public void ManifestChanged_NullOperand_IsFalse() + { + var loaded = SampleManifest(); + Assert.IsFalse(JsonRpcExtensionService.ManifestChanged(null!, loaded)); + Assert.IsFalse(JsonRpcExtensionService.ManifestChanged(loaded, null!)); + } + + // r3-p4-02: the recursive root watcher reports every descendant path. Only a top-level + // / directory or its own //package.json manifest is an + // extension entry; anything deeper (a nested package or a node_modules manifest) must be + // ignored so a nested package.json is not treated as an extension upsert. + [TestMethod] + public void IsTopLevelExtensionChange_ExtensionDirectory_IsTrue() + { + Assert.IsTrue(JsonRpcExtensionService.IsTopLevelExtensionChange(@"C:\root", @"C:\root\my-extension")); + } + + [TestMethod] + public void IsTopLevelExtensionChange_TopLevelManifest_IsTrue() + { + Assert.IsTrue(JsonRpcExtensionService.IsTopLevelExtensionChange(@"C:\root", @"C:\root\my-extension\package.json")); + } + + [TestMethod] + public void IsTopLevelExtensionChange_NestedManifest_IsFalse() + { + // A package.json inside a nested package or under node_modules is two-plus levels + // below the extension directory and is not an extension entry. + Assert.IsFalse(JsonRpcExtensionService.IsTopLevelExtensionChange( + @"C:\root", Path.Combine(@"C:\root", "my-extension", "node_modules", "dep", "package.json"))); + + Assert.IsFalse(JsonRpcExtensionService.IsTopLevelExtensionChange( + @"C:\root", Path.Combine(@"C:\root", "my-extension", "packages", "inner", "package.json"))); + } + + [TestMethod] + public void IsTopLevelExtensionChange_NestedDirectory_IsFalse() + { + Assert.IsFalse(JsonRpcExtensionService.IsTopLevelExtensionChange( + @"C:\root", Path.Combine(@"C:\root", "my-extension", "dist"))); + } + + [TestMethod] + public void IsTopLevelExtensionChange_NonManifestTopLevelFile_IsFalse() + { + // A file that sits at // but is not the manifest is not an entry. + Assert.IsFalse(JsonRpcExtensionService.IsTopLevelExtensionChange( + @"C:\root", Path.Combine(@"C:\root", "my-extension", "index.js"))); + } + + [TestMethod] + public void IsTopLevelExtensionChange_OutsideRoot_IsFalse() + { + Assert.IsFalse(JsonRpcExtensionService.IsTopLevelExtensionChange(@"C:\root", @"C:\other\my-extension\package.json")); + Assert.IsFalse(JsonRpcExtensionService.IsTopLevelExtensionChange(@"C:\root", @"C:\root")); + } + + [TestMethod] + public void IsTopLevelExtensionChange_ForwardSlashesAndCasing_AreHonored() + { + Assert.IsTrue(JsonRpcExtensionService.IsTopLevelExtensionChange("C:/root", "C:/root/my-extension/Package.json")); + Assert.IsFalse(JsonRpcExtensionService.IsTopLevelExtensionChange("C:/root", "C:/root/my-extension/node_modules/dep/package.json")); + } + + [TestMethod] + public void IsTopLevelExtensionChange_Empty_IsFalse() + { + Assert.IsFalse(JsonRpcExtensionService.IsTopLevelExtensionChange(string.Empty, @"C:\root\ext")); + Assert.IsFalse(JsonRpcExtensionService.IsTopLevelExtensionChange(@"C:\root", string.Empty)); + } + + private static JSExtensionManifest SampleManifest() => new() + { + Name = "my-extension", + DisplayName = "My Extension", + Version = "1.0.0", + Description = "A sample extension.", + Icon = "\uE700", + Publisher = "Contoso", + Main = "index.js", + EntryPointPath = @"C:\ext\my-extension\index.js", + Debug = false, + DebugPort = null, + }; +} diff --git a/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/NodeRuntimeLocatorTests.cs b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/NodeRuntimeLocatorTests.cs new file mode 100644 index 0000000000..bfb77effbc --- /dev/null +++ b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/NodeRuntimeLocatorTests.cs @@ -0,0 +1,75 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.IO; +using Microsoft.CmdPal.UI.ViewModels.Services; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Microsoft.CmdPal.UI.ViewModels.UnitTests; + +[TestClass] +public class NodeRuntimeLocatorTests +{ + private static readonly string[] MalformedPathEntries = { "invalid|path" }; + + [TestMethod] + public void ResolveNodeExecutable_ReturnsFirstDirectoryThatContainsNodeExe() + { + var root = Path.Combine(Path.GetTempPath(), "cmdpal-node-locator-" + Guid.NewGuid().ToString("N")); + var withoutNode = Path.Combine(root, "without"); + var withNode = Path.Combine(root, "with"); + Directory.CreateDirectory(withoutNode); + Directory.CreateDirectory(withNode); + + var expected = Path.Combine(withNode, "node.exe"); + + try + { + File.WriteAllText(expected, string.Empty); + + // The first directory has no node.exe, so resolution must skip it and return + // the absolute path from the second directory rather than the bare name. + var resolved = NodeRuntimeLocator.ResolveNodeExecutable(new[] { withoutNode, withNode }); + + Assert.AreEqual(expected, resolved); + } + finally + { + Directory.Delete(root, recursive: true); + } + } + + [TestMethod] + public void ResolveNodeExecutable_ReturnsNullWhenNodeExeIsNotPresent() + { + var root = Path.Combine(Path.GetTempPath(), "cmdpal-node-locator-" + Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(root); + + try + { + var resolved = NodeRuntimeLocator.ResolveNodeExecutable(new[] { root }); + Assert.IsNull(resolved); + } + finally + { + Directory.Delete(root, recursive: true); + } + } + + [TestMethod] + public void ResolveNodeExecutable_SkipsMalformedPathEntries() + { + // A PATH entry containing invalid path characters must be skipped rather than + // throwing, so a single bad entry cannot break node.exe resolution. + var resolved = NodeRuntimeLocator.ResolveNodeExecutable(MalformedPathEntries); + Assert.IsNull(resolved); + } + + [TestMethod] + public void ResolveNodeExecutable_ReturnsNullForEmptyDirectoryList() + { + Assert.IsNull(NodeRuntimeLocator.ResolveNodeExecutable(Array.Empty())); + } +} diff --git a/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/ProviderIdReservationsTests.cs b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/ProviderIdReservationsTests.cs new file mode 100644 index 0000000000..0d86dcbe91 --- /dev/null +++ b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/ProviderIdReservationsTests.cs @@ -0,0 +1,132 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System.Collections.Concurrent; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CmdPal.UI.ViewModels.Services; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Microsoft.CmdPal.UI.ViewModels.UnitTests; + +/// +/// Verifies the atomic provider-id reservation registry (r2-p4-01). A provider id can be +/// owned by exactly one directory at a time regardless of how many registration paths +/// race for it (full scan, hot reload, or concurrent install), and it is freed only by +/// its owner so a stale release cannot steal an id that was reclaimed by someone else. +/// +[TestClass] +public class ProviderIdReservationsTests +{ + [TestMethod] + public void TryReserve_FreeId_Succeeds() + { + var reservations = new ProviderIdReservations(); + Assert.IsTrue(reservations.TryReserve("alpha", @"C:\ext\a")); + } + + [TestMethod] + public void TryReserve_SameDirectoryAgain_IsIdempotent() + { + var reservations = new ProviderIdReservations(); + Assert.IsTrue(reservations.TryReserve("alpha", @"C:\ext\a")); + + // The same owner re-reserving its own id must succeed (a hot-reload re-registers + // the same directory). Callers pass canonical keys, which compare case-insensitively + // to match the rest of the service's directory comparisons. + Assert.IsTrue(reservations.TryReserve("alpha", @"C:\ext\a")); + Assert.IsTrue(reservations.TryReserve("alpha", @"c:\ext\A")); + } + + [TestMethod] + public void TryReserve_DifferentDirectorySameId_Fails() + { + var reservations = new ProviderIdReservations(); + Assert.IsTrue(reservations.TryReserve("alpha", @"C:\ext\a")); + Assert.IsFalse(reservations.TryReserve("alpha", @"C:\ext\b")); + } + + [TestMethod] + public void TryReserve_EmptyId_AlwaysSucceedsAndReservesNothing() + { + var reservations = new ProviderIdReservations(); + + // An extension with no name key has nothing to collide on; two different + // directories can both "reserve" an empty id. + Assert.IsTrue(reservations.TryReserve(string.Empty, @"C:\ext\a")); + Assert.IsTrue(reservations.TryReserve(string.Empty, @"C:\ext\b")); + Assert.IsTrue(reservations.TryReserve(null, @"C:\ext\c")); + } + + [TestMethod] + public void Release_ByOwner_FreesId() + { + var reservations = new ProviderIdReservations(); + Assert.IsTrue(reservations.TryReserve("alpha", @"C:\ext\a")); + + reservations.Release("alpha", @"C:\ext\a"); + + // Now a different directory may claim it. + Assert.IsTrue(reservations.TryReserve("alpha", @"C:\ext\b")); + } + + [TestMethod] + public void Release_ByNonOwner_DoesNotFreeId() + { + var reservations = new ProviderIdReservations(); + Assert.IsTrue(reservations.TryReserve("alpha", @"C:\ext\a")); + + // A stale release from a directory that does not own the id must be ignored, so + // the real owner keeps it. + reservations.Release("alpha", @"C:\ext\b"); + + Assert.IsFalse(reservations.TryReserve("alpha", @"C:\ext\b")); + Assert.IsTrue(reservations.TryReserve("alpha", @"C:\ext\a")); + } + + [TestMethod] + public void Clear_ReleasesEverything() + { + var reservations = new ProviderIdReservations(); + Assert.IsTrue(reservations.TryReserve("alpha", @"C:\ext\a")); + Assert.IsTrue(reservations.TryReserve("beta", @"C:\ext\b")); + + reservations.Clear(); + + Assert.IsTrue(reservations.TryReserve("alpha", @"C:\ext\x")); + Assert.IsTrue(reservations.TryReserve("beta", @"C:\ext\y")); + } + + [TestMethod] + public async Task TryReserve_ConcurrentDifferentDirectories_ExactlyOneWins() + { + var reservations = new ProviderIdReservations(); + const int Contenders = 32; + + using var start = new ManualResetEventSlim(false); + var winners = new ConcurrentBag(); + var tasks = new Task[Contenders]; + + for (var i = 0; i < Contenders; i++) + { + var index = i; + tasks[i] = Task.Run(() => + { + // Every contender blocks on the same gate so they all race the reservation + // at once, simulating a full scan, a hot reload, and a concurrent install + // all claiming the same id. + start.Wait(); + if (reservations.TryReserve("shared-id", $@"C:\ext\dir-{index}")) + { + winners.Add(index); + } + }); + } + + start.Set(); + await Task.WhenAll(tasks); + + Assert.AreEqual(1, winners.Count, "Exactly one directory may claim a provider id, regardless of the registration path."); + } +} diff --git a/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/RecordingExtensionHost.cs b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/RecordingExtensionHost.cs new file mode 100644 index 0000000000..f1efcd8d82 --- /dev/null +++ b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/RecordingExtensionHost.cs @@ -0,0 +1,139 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CommandPalette.Extensions; +using Windows.Foundation; + +namespace Microsoft.CmdPal.UI.ViewModels.UnitTests; + +/// +/// An that records every status and log call in arrival +/// order and lets a test await a specific number of calls without polling. Used by the +/// proxy startup-notification and status-lifecycle tests. +/// +internal sealed partial class RecordingExtensionHost : IExtensionHost +{ + private static readonly TimeSpan DefaultTimeout = TimeSpan.FromSeconds(10); + + private readonly Lock _gate = new(); + private readonly List _shown = []; + private readonly List _hidden = []; + private readonly List _logs = []; + private readonly List<(int Target, TaskCompletionSource Signal)> _shownWaiters = []; + private readonly List<(int Target, TaskCompletionSource Signal)> _hiddenWaiters = []; + private readonly List<(int Target, TaskCompletionSource Signal)> _logWaiters = []; + + public IReadOnlyList Shown + { + get + { + lock (_gate) + { + return [.. _shown]; + } + } + } + + public IReadOnlyList Hidden + { + get + { + lock (_gate) + { + return [.. _hidden]; + } + } + } + + public IReadOnlyList Logs + { + get + { + lock (_gate) + { + return [.. _logs]; + } + } + } + + public Task WaitForShownCountAsync(int count) => WaitFor(_shown, _shownWaiters, count); + + public Task WaitForHiddenCountAsync(int count) => WaitFor(_hidden, _hiddenWaiters, count); + + public Task WaitForLogCountAsync(int count) => WaitFor(_logs, _logWaiters, count); + + public IAsyncAction ShowStatus(IStatusMessage? message, StatusContext context) + { + if (message is not null) + { + Record(_shown, _shownWaiters, message); + } + + return Task.CompletedTask.AsAsyncAction(); + } + + public IAsyncAction HideStatus(IStatusMessage? message) + { + if (message is not null) + { + Record(_hidden, _hiddenWaiters, message); + } + + return Task.CompletedTask.AsAsyncAction(); + } + + public IAsyncAction LogMessage(ILogMessage? message) + { + if (message is not null) + { + Record(_logs, _logWaiters, message); + } + + return Task.CompletedTask.AsAsyncAction(); + } + + private void Record(List sink, List<(int Target, TaskCompletionSource Signal)> waiters, T value) + { + List ready = []; + lock (_gate) + { + sink.Add(value); + var current = sink.Count; + for (var i = waiters.Count - 1; i >= 0; i--) + { + if (current >= waiters[i].Target) + { + ready.Add(waiters[i].Signal); + waiters.RemoveAt(i); + } + } + } + + foreach (var signal in ready) + { + signal.TrySetResult(); + } + } + + private Task WaitFor(List sink, List<(int Target, TaskCompletionSource Signal)> waiters, int count) + { + TaskCompletionSource signal; + lock (_gate) + { + if (sink.Count >= count) + { + return Task.CompletedTask; + } + + signal = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + waiters.Add((count, signal)); + } + + return signal.Task.WaitAsync(DefaultTimeout); + } +} diff --git a/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/ReloadCancellationTests.cs b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/ReloadCancellationTests.cs new file mode 100644 index 0000000000..cb1de560bf --- /dev/null +++ b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/ReloadCancellationTests.cs @@ -0,0 +1,66 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using Microsoft.CmdPal.UI.ViewModels.Services; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Microsoft.CmdPal.UI.ViewModels.UnitTests; + +/// +/// Verifies the replaceable reload cancellation source (p4-01). A single +/// CancellationTokenSource can only cancel once, so a service that shares one token +/// between stop and a later reload would keep handing out an already canceled token. +/// These tests confirm a fresh, uncanceled token is available after a stop. +/// +[TestClass] +public class ReloadCancellationTests +{ + [TestMethod] + public void BeginCycle_AfterStop_YieldsFreshUncanceledToken() + { + using var reload = new ReloadCancellation(); + + var first = reload.BeginCycle(); + Assert.IsFalse(first.IsCancellationRequested, "A new cycle should start uncanceled."); + + reload.Stop(); + Assert.IsTrue(reload.IsStopRequested, "A stop should be observable."); + Assert.IsTrue(first.IsCancellationRequested, "In-flight token should observe the stop."); + + // The load-stop-load sequence: a second cycle must produce a live token so + // providers load on the second load instead of silently doing nothing. + var second = reload.BeginCycle(); + Assert.IsFalse(second.IsCancellationRequested, "The second load cycle should get a live token."); + Assert.IsFalse(reload.IsStopRequested, "The wrapper should no longer report a stop after a new cycle."); + } + + [TestMethod] + public void Token_AfterDispose_IsCanceledAndDoesNotThrow() + { + var reload = new ReloadCancellation(); + reload.BeginCycle(); + reload.Dispose(); + + Assert.IsTrue(reload.IsStopRequested); + Assert.IsTrue(reload.Token.IsCancellationRequested); + + // Begin after dispose returns a canceled token rather than throwing. + Assert.IsTrue(reload.BeginCycle().IsCancellationRequested); + + // Dispose is idempotent. + reload.Dispose(); + } + + [TestMethod] + public void Stop_WithoutBeginCycle_StillCancelsCurrentToken() + { + using var reload = new ReloadCancellation(); + + var token = reload.Token; + reload.Stop(); + + Assert.IsTrue(token.IsCancellationRequested); + Assert.IsTrue(reload.IsStopRequested); + } +} diff --git a/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/SerialNotificationDispatcherTests.cs b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/SerialNotificationDispatcherTests.cs new file mode 100644 index 0000000000..e7a59cf9a1 --- /dev/null +++ b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/SerialNotificationDispatcherTests.cs @@ -0,0 +1,129 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Collections.Concurrent; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CmdPal.UI.ViewModels.Services; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Microsoft.CmdPal.UI.ViewModels.UnitTests; + +/// +/// Verifies the single ordered dispatch path for provider add/remove notifications +/// (r3-p4-04). Every emission runs on one worker in strict first-in-first-out order, so a +/// consumer can never observe a provider addition ahead of the removal enqueued before it, +/// even when the two originate on different threads. +/// +[TestClass] +public class SerialNotificationDispatcherTests +{ + [TestMethod] + public void Enqueue_RunsNotificationsInFifoOrder() + { + using var dispatcher = new SerialNotificationDispatcher(); + var observed = new ConcurrentQueue(); + var done = new CountdownEvent(500); + + for (var i = 0; i < 500; i++) + { + var value = i; + dispatcher.Enqueue(() => + { + observed.Enqueue(value); + done.Signal(); + }); + } + + Assert.IsTrue(done.Wait(TimeSpan.FromSeconds(5)), "All notifications should have run."); + + var expected = 0; + foreach (var value in observed) + { + Assert.AreEqual(expected, value, "Notifications must run in enqueue order."); + expected++; + } + + Assert.AreEqual(500, expected); + } + + // A paired removal enqueued ahead of an addition must always be observed first, even + // when the two are enqueued from different threads racing each other. + [TestMethod] + public void Enqueue_FromConcurrentThreads_PreservesPerCallerOrder() + { + using var dispatcher = new SerialNotificationDispatcher(); + var removeBeforeAdd = true; + var addSeen = false; + var done = new CountdownEvent(200); + + for (var i = 0; i < 100; i++) + { + // Each iteration enqueues a "remove" then an "add" from the same caller. The + // add handler must never run before its paired remove handler. + var removed = false; + dispatcher.Enqueue(() => + { + removed = true; + done.Signal(); + }); + dispatcher.Enqueue(() => + { + if (!removed) + { + removeBeforeAdd = false; + } + + addSeen = true; + done.Signal(); + }); + } + + Assert.IsTrue(done.Wait(TimeSpan.FromSeconds(5)), "All notifications should have run."); + Assert.IsTrue(addSeen); + Assert.IsTrue(removeBeforeAdd, "An addition must never overtake the removal enqueued before it."); + } + + [TestMethod] + public void Enqueue_AfterDispose_IsDroppedSilently() + { + var dispatcher = new SerialNotificationDispatcher(); + dispatcher.Dispose(); + + var ran = false; + dispatcher.Enqueue(() => ran = true); + + Thread.Sleep(100); + Assert.IsFalse(ran, "A notification enqueued after dispose must not run."); + } + + [TestMethod] + public void Dispose_DrainsAlreadyEnqueuedNotifications() + { + var dispatcher = new SerialNotificationDispatcher(); + var count = 0; + + for (var i = 0; i < 50; i++) + { + dispatcher.Enqueue(() => Interlocked.Increment(ref count)); + } + + dispatcher.Dispose(); + + Assert.AreEqual(50, Volatile.Read(ref count), "Dispose must let already-queued notifications drain."); + } + + [TestMethod] + public void Enqueue_HandlerException_DoesNotStopLaterNotifications() + { + using var dispatcher = new SerialNotificationDispatcher(); + var reached = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + dispatcher.Enqueue(() => throw new InvalidOperationException("boom")); + dispatcher.Enqueue(() => reached.TrySetResult()); + + Assert.IsTrue(reached.Task.Wait(TimeSpan.FromSeconds(5)), "A throwing handler must not stall the worker."); + } +}