Address code review feedback: optimize registry lookups, add input validation, improve diagnostics

docs/engine-overview.md

@@ -7,9 +7,10 @@ This repository now exposes transport-agnostic engines that sit inside each modu
 - **Engine manifest** (`ModuleEngineManifest`)
   - Identifies the engine (`Id`, `Version`, `Description`, `Kind`, optional `FeatureArea`).
   - Declares capabilities, schemas, security, compatibility expectations, and navigation hints via strongly typed records/enums (`ModuleEngineNavigationHints`, `ModuleNavigationToken`, `NavigationTargetKind`, `ModuleSignatureAlgorithm`).
+  - Declares `RequiredServices` – a list of logical capabilities (for example `IApiClient`, `ICache`, `ITelemetry`) that must be provided by the host for the engine to run. Hosts are expected to validate that every required service can be satisfied before executing an engine, typically by mapping each required service identifier to a concrete implementation in their DI container or adapter configuration. If validation fails, hosts should skip registration or surface a configuration error rather than invoking the engine.
   - Adds optional webhook metadata (`ModuleEngineWebhookMetadata`) for providers and event types.
 - **Engine descriptor** (`ModuleEngineDescriptor<TContract>`) wraps the manifest with a strongly typed factory so hosts can resolve engines without `object` casts.
-- **Discovery** (`ModuleEngineDiscoveryService`/`ModuleEngineRegistry`) stores descriptors and supports filtering by kind/feature or matching webhook provider + event type. Hosts can either enumerate engines (dynamic) or resolve known descriptors directly (static, isolation-focused deployments).
+- **Discovery** (`ModuleEngineDiscoveryService`/`ModuleEngineRegistry`) stores descriptors and supports filtering by kind/feature or matching webhook provider + event type. Hosts can either enumerate engines (dynamic) or resolve known descriptors directly (static, isolation-focused deployments), and use the exposed manifests (including `RequiredServices`) to ensure all dependencies are available before wiring engines into request or worker pipelines.
 
 ## Adapter roles
 

src/Bravellian.Platform.Modularity.Api/WebhookEngineAdapter.cs

@@ -38,6 +38,21 @@ public WebhookEngineAdapter(ModuleEngineDiscoveryService discovery, IServiceProv
     /// </summary>
     public async Task<WebhookAdapterResponse> DispatchAsync<TPayload>(WebhookAdapterRequest<TPayload> request, CancellationToken cancellationToken)
     {
+        if (request is null)
+        {
+            throw new ArgumentNullException(nameof(request));
+        }
+
+        if (string.IsNullOrWhiteSpace(request.Provider))
+        {
+            throw new ArgumentException("Provider must be a non-empty, non-whitespace string.", nameof(request));
+        }
+
+        if (string.IsNullOrWhiteSpace(request.EventType))
+        {
+            throw new ArgumentException("EventType must be a non-empty, non-whitespace string.", nameof(request));
+        }
+
         var descriptor = discovery.ResolveWebhookEngine(request.Provider, request.EventType)
             ?? throw new InvalidOperationException($"No webhook engine registered for provider '{request.Provider}' and event '{request.EventType}'.");
 
@@ -49,7 +64,9 @@ public async Task<WebhookAdapterResponse> DispatchAsync<TPayload>(WebhookAdapter
 
         if (string.IsNullOrWhiteSpace(request.IdempotencyKey) && descriptor.Manifest.Security?.IdempotencyWindow is not null)
         {
-            return new WebhookAdapterResponse(WebhookOutcomeType.Retry, "Missing idempotency key");
+            return new WebhookAdapterResponse(
+                WebhookOutcomeType.Retry,
+                "Idempotency key is required when an idempotency window is configured for this provider.");
         }
 
         var typedDescriptor = descriptor as ModuleEngineDescriptor<IWebhookEngine<TPayload>>

src/Bravellian.Platform.Modularity.Core/ModuleEngineCapabilities.cs

@@ -19,7 +19,7 @@ namespace Bravellian.Platform.Modularity;
 /// </summary>
 /// <param name="Actions">Named commands/actions the engine supports.</param>
 /// <param name="Events">Events emitted by the engine.</param>
-/// <param name="SupportsAsync">Indicates async execution is supported.</param>
+/// <param name="SupportsAsync">Reserved for future use. All engines are currently async by design (Task-based). This flag may be used by hosts to differentiate execution strategies in future versions.</param>
 /// <param name="SupportsStreaming">Indicates streaming updates are supported.</param>
 public sealed record ModuleEngineCapabilities(
     IReadOnlyCollection<string> Actions,

src/Bravellian.Platform.Modularity.Core/ModuleEngineRegistry.cs

@@ -59,19 +59,59 @@ public static IReadOnlyCollection<IModuleEngineDescriptor> GetEngines()
 
     public static IModuleEngineDescriptor? FindWebhookEngine(string provider, string eventType)
     {
-        return GetEngines()
-            .Where(e => e.Manifest.Kind == EngineKind.Webhook)
-            .SelectMany(descriptor => descriptor.Manifest.WebhookMetadata?.Select(meta => (descriptor, meta))
-                                   ?? Array.Empty<(IModuleEngineDescriptor descriptor, ModuleEngineWebhookMetadata meta)>())
-            .FirstOrDefault(pair => string.Equals(pair.meta.Provider, provider, StringComparison.OrdinalIgnoreCase)
-                                 && string.Equals(pair.meta.EventType, eventType, StringComparison.OrdinalIgnoreCase))
-            .descriptor;
+        // Avoid creating a full snapshot; search lists directly under their locks.
+        foreach (var list in Engines.Values)
+        {
+            lock (list)
+            {
+                foreach (var descriptor in list)
+                {
+                    if (descriptor.Manifest.Kind != EngineKind.Webhook)
+                    {
+                        continue;
+                    }
+
+                    var metadataCollection = descriptor.Manifest.WebhookMetadata;
+                    if (metadataCollection == null)
+                    {
+                        continue;
+                    }
+
+                    foreach (var meta in metadataCollection)
+                    {
+                        if (string.Equals(meta.Provider, provider, StringComparison.OrdinalIgnoreCase)
+                            && string.Equals(meta.EventType, eventType, StringComparison.OrdinalIgnoreCase))
+                        {
+                            return descriptor;
+                        }
+                    }
+                }
+            }
+        }
+
+        return null;
     }
 
     public static IModuleEngineDescriptor? FindById(string moduleKey, string engineId)
     {
-        return GetEngines().FirstOrDefault(e => string.Equals(e.ModuleKey, moduleKey, StringComparison.OrdinalIgnoreCase)
-                                             && string.Equals(e.Manifest.Id, engineId, StringComparison.OrdinalIgnoreCase));
+        // Narrow the lookup to the specific moduleKey instead of scanning all engines.
+        if (!Engines.TryGetValue(moduleKey, out var list))
+        {
+            return null;
+        }
+
+        lock (list)
+        {
+            foreach (var descriptor in list)
+            {
+                if (string.Equals(descriptor.Manifest.Id, engineId, StringComparison.OrdinalIgnoreCase))
+                {
+                    return descriptor;
+                }
+            }
+        }
+
+        return null;
     }
 
     public static void Reset()

src/Bravellian.Platform.Modularity.Core/ModuleRegistry.cs

@@ -89,7 +89,10 @@ internal static IReadOnlyCollection<TModule> InitializeModules<TModule>(
                     {
                         if (!string.Equals(descriptor.ModuleKey, module.Key, StringComparison.OrdinalIgnoreCase))
                         {
-                            throw new InvalidOperationException($"Engine descriptor module key '{descriptor.ModuleKey}' must match module '{module.Key}'.");
+                            throw new InvalidOperationException(
+                                $"Engine descriptor module key '{descriptor.ModuleKey}' must match its owning module key '{module.Key}'. " +
+                                "Engine descriptors must use their owning module's key to ensure proper isolation and discovery. " +
+                                "Update the engine descriptor's ModuleKey to match the module's Key.");
                         }
 
                         return descriptor;

src/Bravellian.Platform.Modularity.Core/WebhookRequest.cs

@@ -21,7 +21,7 @@ namespace Bravellian.Platform.Modularity;
 /// <param name="EventType">Event type identifier.</param>
 /// <param name="Payload">Deserialized payload.</param>
 /// <param name="IdempotencyKey">Idempotency key for replay protection.</param>
-/// <param name="Attempt">Current delivery attempt.</param>
+/// <param name="Attempt">Current delivery attempt (1-based; default is 0 for backwards compatibility, but callers should use 1 for the first attempt).</param>
 public sealed record WebhookRequest<TPayload>(
     string Provider,
     string EventType,

src/Bravellian.Platform.Modularity.FullStack/UiEngineAdapter.cs

@@ -36,6 +36,16 @@ public UiEngineAdapter(ModuleEngineDiscoveryService discoveryService, IServicePr
     /// </summary>
     public async Task<UiAdapterResponse<TViewModel>> ExecuteAsync<TInput, TViewModel>(string moduleKey, string engineId, TInput command, CancellationToken cancellationToken)
     {
+        if (string.IsNullOrWhiteSpace(moduleKey))
+        {
+            throw new ArgumentException("Module key must be a non-empty, non-whitespace string.", nameof(moduleKey));
+        }
+
+        if (string.IsNullOrWhiteSpace(engineId))
+        {
+            throw new ArgumentException("Engine ID must be a non-empty, non-whitespace string.", nameof(engineId));
+        }
+
         var descriptor = discoveryService.ResolveById(moduleKey, engineId)
             ?? throw new InvalidOperationException($"No UI engine registered with id '{engineId}' for module '{moduleKey}'.");
 

tests/Bravellian.Platform.Tests/EngineRefactoringTests.cs

@@ -62,6 +62,60 @@ public async Task Webhook_adapter_enforces_signature_and_maps_outcome()
         Assert.Equal(WebhookOutcomeType.EnqueueEvent, response.Outcome);
     }
 
+    [Fact]
+    public async Task Webhook_adapter_rejects_invalid_signature()
+    {
+        var provider = BuildServiceProvider();
+        var adapter = new WebhookEngineAdapter(provider.GetRequiredService<ModuleEngineDiscoveryService>(), provider, provider.GetRequiredService<IWebhookSignatureValidator>());
+
+        var request = new WebhookAdapterRequest<PostmarkBouncePayload>(
+            "postmark",
+            "bounce",
+            new Dictionary<string, string> { ["X-Signature"] = "invalid-signature" },
+            "raw-body",
+            "idemp-1",
+            1,
+            null,
+            new PostmarkBouncePayload("HardBounce", "Mail rejected"));
+
+        var response = await adapter.DispatchAsync(request, CancellationToken.None);
+
+        Assert.Equal(WebhookOutcomeType.Acknowledge, response.Outcome);
+        Assert.Equal("Signature validation failed", response.Reason);
+    }
+
+    [Fact]
+    public async Task Webhook_adapter_requires_idempotency_key_when_configured()
+    {
+        var provider = BuildServiceProvider();
+        var adapter = new WebhookEngineAdapter(provider.GetRequiredService<ModuleEngineDiscoveryService>(), provider, provider.GetRequiredService<IWebhookSignatureValidator>());
+
+        var request = new WebhookAdapterRequest<PostmarkBouncePayload>(
+            "postmark",
+            "bounce",
+            new Dictionary<string, string> { ["X-Signature"] = "postmark:raw-body" },
+            "raw-body",
+            string.Empty, // Missing idempotency key
+            1,
+            null,
+            new PostmarkBouncePayload("HardBounce", "Mail rejected"));
+
+        var response = await adapter.DispatchAsync(request, CancellationToken.None);
+
+        Assert.Equal(WebhookOutcomeType.Retry, response.Outcome);
+        Assert.Contains("Idempotency key is required", response.Reason);
+    }
+
+    [Fact]
+    public async Task Ui_engine_exception_propagates_to_adapter()
+    {
+        var provider = BuildServiceProvider();
+        var adapter = new UiEngineAdapter(provider.GetRequiredService<ModuleEngineDiscoveryService>(), provider);
+
+        await Assert.ThrowsAsync<ArgumentException>(async () =>
+            await adapter.ExecuteAsync<LoginCommand, LoginViewModel>("fake-module", "ui.login", new LoginCommand(string.Empty, "pass"), CancellationToken.None));
+    }
+
     [Fact]
     public void Discovery_service_filters_engines()
     {