feat: implement module scheduling constraints and enhance scheduler (#1304)

* refactor: enhance module execution with eager parallel scheduling and improved state management

* feat: implement module scheduling constraints and enhance scheduler functionality

* refactor: optimize blocking module search by converting values to a list

* refactor: improve module state properties with volatile fields for thread safety

* refactor: unify module state management by replacing boolean flags with an enum for clarity and consistency

* feat: add detailed logging for module execution and queuing states

* refactor: enhance constraint checking in module execution with improved logging and state consistency

* fix: improve constraint checking with state snapshot and comprehensive debug logging

- Snapshot all module states before filtering to ensure consistent view
- Add detailed debug logging for constraint checking to diagnose race conditions
- Change all diagnostic logs to LogDebug level
- Log active module comparisons, lock key intersections, and blocking decisions

* fix: exclude self from constraint checks in module scheduling

* fix: use tracking collections for constraint checking

Fixes race condition in constraint checking by using the tracking
collections (_queuedModules and _executingModules) directly instead
of filtering by State enum. This prevents the issue where State changes
during iteration would be immediately visible through list references,
causing inconsistent constraint evaluation.

Changes:
- Replace state-based filtering with direct collection usage
- Add self-exclusion checks to prevent modules comparing against themselves
- Remove snapshot approach that created references to mutable state

This ensures modules with conflicting lock keys are properly blocked
from parallel execution, preventing the race condition that allowed
Module2 and Module3 (both with lock key "B") to execute concurrently.

Fixes #1304

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>

* fix: simplify long-running module execution by removing timeout handling

* fix: respect engine cancellation in module execution task handling

* fix: add timeout handling for LongRunningModule to ensure cancellation behavior

* fix: correct await usage in timeout handling for module execution

* fix: prevent unobserved task exceptions by observing executeAsyncTask's exception in timeout handling

* fix: add defensive constraint check at execution boundary to prevent race conditions

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: thomhurst

src/ModularPipelines/Context/IPipelineHookContext.cs

@@ -11,76 +11,135 @@ namespace ModularPipelines.Context;
 /// <summary>
 /// The pipeline context, with access to configuration, DI and helper objects.
 /// </summary>
+/// <remarks>
+/// This interface serves as a facade providing unified access to all pipeline capabilities.
+/// Each property represents a focused helper for a specific domain (file system, commands, serialization, etc.).
+///
+/// The facade pattern is intentional here to:
+/// - Provide a single entry point for all pipeline operations
+/// - Allow dependency injection of all helpers
+/// - Support extension methods for tool-specific integrations (Git, Docker, Azure, etc.)
+///
+/// Extension Pattern:
+/// Tool integrations (ModularPipelines.Git, ModularPipelines.Docker, etc.) add extension methods
+/// like context.Git() and context.Docker() that provide strongly-typed APIs for those tools.
+/// </remarks>
 public interface IPipelineHookContext
 {
+    #region DependencyInjection
+
     /// <summary>
     /// Gets the service provider orchestrating DI within the pipeline.
     /// </summary>
+    /// <remarks>
+    /// Use the generic Get&lt;T&gt;() method instead of accessing ServiceProvider directly
+    /// when possible for better type safety.
+    /// </remarks>
     public IServiceProvider ServiceProvider { get; }
 
+    /// <summary>
+    /// Helper method for retrieving services from the Service Provider.
+    /// </summary>
+    /// <typeparam name="T">Type to retrieve.</typeparam>
+    /// <returns>The service instance, or null if not registered.</returns>
+    public T? Get<T>();
+
+    #endregion
+
+    #region Configuration
+
     /// <summary>
     /// Gets the configuration powering the pipeline.
     /// </summary>
+    /// <remarks>
+    /// Configuration is loaded from appsettings.json, environment variables, user secrets, and command line.
+    /// </remarks>
     public IConfiguration Configuration { get; }
 
     /// <summary>
     /// Gets the pipeline's options.
     /// </summary>
     public IOptions<PipelineOptions> PipelineOptions { get; }
 
-    /// <summary>
-    /// Helper method for retrieving services from the Service Provider.
-    /// </summary>
-    /// <typeparam name="T">Type to retrieve.</typeparam>
-    /// <returns>T.</returns>
-    public T? Get<T>();
+    #endregion
+
+    #region Logging
 
     /// <summary>
     /// Gets the logger to be used from the pipeline.
     /// </summary>
+    /// <remarks>
+    /// This logger is module-scoped and will include the module name in log output.
+    /// </remarks>
     public IModuleLogger Logger { get; }
 
-    #region OutOfTheBoxHelpers
+    #endregion
+
+    #region EnvironmentHelpers
 
     /// <summary>
     /// Gets helpers for getting information about the current environment.
     /// </summary>
+    /// <remarks>
+    /// Provides access to OS info, environment variables, working directory, etc.
+    /// </remarks>
     public IEnvironmentContext Environment { get; }
 
     /// <summary>
-    /// Gets helpers for interacting with the filesystem.
+    /// Gets helpers for detecting the current build system.
     /// </summary>
-    public IFileSystemContext FileSystem { get; }
+    /// <remarks>
+    /// Detects GitHub Actions, Azure DevOps, Jenkins, GitLab CI, etc.
+    /// </remarks>
+    public IBuildSystemDetector BuildSystemDetector { get; }
 
-    /// <summary>
-    /// Gets helpers for running commands.
-    /// </summary>
-    public ICommand Command { get; }
+    #endregion
+
+    #region FileSystemHelpers
 
     /// <summary>
-    /// Gets helpers for installing applications.
+    /// Gets helpers for interacting with the filesystem.
     /// </summary>
-    public IInstaller Installer { get; }
+    /// <remarks>
+    /// Provides file/directory operations with logging and error handling.
+    /// </remarks>
+    public IFileSystemContext FileSystem { get; }
 
     /// <summary>
     /// Gets helpers for zipping and unzipping files and directories.
     /// </summary>
     public IZip Zip { get; }
 
     /// <summary>
-    /// Gets helpers for converting to and from hex strings.
+    /// Gets helpers for checking the Checksum of a file.
     /// </summary>
-    public IHex Hex { get; }
+    IChecksum Checksum { get; }
+
+    #endregion
+
+    #region CommandExecution
 
     /// <summary>
-    /// Gets helpers for converting to and from Base64 strings.
+    /// Gets helpers for running commands.
     /// </summary>
-    public IBase64 Base64 { get; }
+    /// <remarks>
+    /// Provides cross-platform command execution with output capture and logging.
+    /// </remarks>
+    public ICommand Command { get; }
 
     /// <summary>
-    /// Gets helpers for hashing data.
+    /// Gets helpers for running powershell.
     /// </summary>
-    public IHasher Hasher { get; }
+    public IPowershell Powershell { get; }
+
+    /// <summary>
+    /// Gets helpers for executing bash commands.
+    /// </summary>
+    public IBash Bash { get; }
+
+    #endregion
+
+    #region Serialization
 
     /// <summary>
     /// Gets helpers for converting JSON.
@@ -97,38 +156,68 @@ public interface IPipelineHookContext
     /// </summary>
     public IYaml Yaml { get; }
 
+    #endregion
+
+    #region Encoding
+
     /// <summary>
-    /// Gets helpers for running powershell.
+    /// Gets helpers for converting to and from hex strings.
     /// </summary>
-    public IPowershell Powershell { get; }
+    public IHex Hex { get; }
 
     /// <summary>
-    /// Gets helpers for executing bash commands.
+    /// Gets helpers for converting to and from Base64 strings.
     /// </summary>
-    public IBash Bash { get; }
+    public IBase64 Base64 { get; }
+
+    #endregion
+
+    #region Security
 
     /// <summary>
-    /// Gets helpers for detecting the current build system.
+    /// Gets helpers for hashing data.
     /// </summary>
-    public IBuildSystemDetector BuildSystemDetector { get; }
+    /// <remarks>
+    /// Supports MD5, SHA1, SHA256, SHA512 hashing algorithms.
+    /// </remarks>
+    public IHasher Hasher { get; }
+
+    #endregion
+
+    #region NetworkHelpers
 
     /// <summary>
     /// Gets helpers for sending HTTP requests.
     /// </summary>
+    /// <remarks>
+    /// Provides a configured HttpClient with retry policies and logging.
+    /// </remarks>
     public IHttp Http { get; }
 
     /// <summary>
     /// Gets helpers for downloading data from the web.
     /// </summary>
+    /// <remarks>
+    /// Includes progress tracking and verification helpers.
+    /// </remarks>
     public IDownloader Downloader { get; }
 
+    #endregion
+
+    #region Installation
+
     /// <summary>
-    /// Gets helpers for checking the Checksum of a file.
+    /// Gets helpers for installing applications.
     /// </summary>
-    IChecksum Checksum { get; }
+    /// <remarks>
+    /// Supports package managers like Chocolatey, apt, brew, etc.
+    /// </remarks>
+    public IInstaller Installer { get; }
 
     #endregion
 
+    #region Internal
+
     /// <summary>
     /// Gets the detector used to detect modules which depend on each other.
     /// </summary>
@@ -149,4 +238,6 @@ public interface IPipelineHookContext
     /// Gets the cancellation token used for cancelling the pipeline on failures.
     /// </summary>
     internal EngineCancellationToken EngineCancellationToken { get; }
+
+    #endregion
 }

src/ModularPipelines/DependencyInjection/DependencyInjectionSetup.cs

@@ -25,6 +25,7 @@ public static void Initialize(IServiceCollection services)
         // Bundles
         services
             .Configure<PipelineOptions>(_ => { })
+            .Configure<SchedulerOptions>(_ => { })
             .AddLogging(builder =>
             {
                 builder.ClearProviders();
@@ -130,6 +131,7 @@ public static void Initialize(IServiceCollection services)
             .AddSingleton<IModuleDisposeExecutor, ModuleDisposeExecutor>()
             .AddSingleton<IPipelineExecutor, PipelineExecutor>()
             .AddSingleton<IModuleExecutor, ModuleExecutor>()
+            .AddSingleton<IModuleSchedulerFactory, ModuleSchedulerFactory>()
             .AddSingleton<IModuleDisposer, ModuleDisposer>()
             .AddSingleton<ILogoPrinter, LogoPrinter>()
             .AddSingleton<IModuleResultRepository, NoOpModuleResultRepository>()

src/ModularPipelines/Engine/Constraints/IModuleConstraint.cs

@@ -0,0 +1,32 @@
+using Microsoft.Extensions.Logging;
+
+namespace ModularPipelines.Engine.Constraints;
+
+/// <summary>
+/// Interface for checking module execution constraints
+/// </summary>
+/// <remarks>
+/// Constraints determine whether a module can execute based on current system state.
+/// Examples include sequential execution requirements, lock key conflicts, etc.
+/// </remarks>
+internal interface IModuleConstraint
+{
+    /// <summary>
+    /// Checks if a module can execute given current state
+    /// </summary>
+    /// <param name="moduleState">The module to check</param>
+    /// <param name="stateQueries">Helper for querying module states</param>
+    /// <returns>True if constraint is satisfied and module can execute, false otherwise</returns>
+    /// <remarks>
+    /// This method MUST be called while holding the scheduler's state lock
+    /// </remarks>
+    bool CanExecute(ModuleState moduleState, ModuleStateQueries stateQueries);
+
+    /// <summary>
+    /// Logs why the constraint blocked execution (called only when CanExecute returns false)
+    /// </summary>
+    /// <param name="moduleState">The module that was blocked</param>
+    /// <param name="stateQueries">Helper for querying module states</param>
+    /// <param name="logger">Logger for output</param>
+    void LogViolation(ModuleState moduleState, ModuleStateQueries stateQueries, ILogger logger);
+}

src/ModularPipelines/Engine/Constraints/LockKeyConstraint.cs

@@ -0,0 +1,39 @@
+using Microsoft.Extensions.Logging;
+using ModularPipelines.Helpers;
+using ModularPipelines.Logging;
+
+namespace ModularPipelines.Engine.Constraints;
+
+/// <summary>
+/// Enforces lock key constraints (NotInParallel attribute with specific keys)
+/// </summary>
+internal class LockKeyConstraint : IModuleConstraint
+{
+    public bool CanExecute(ModuleState moduleState, ModuleStateQueries stateQueries)
+    {
+        if (moduleState.RequiredLockKeys.Length == 0)
+        {
+            return true;
+        }
+
+        return stateQueries.FindModuleWithLockConflict(moduleState) == null;
+    }
+
+    public void LogViolation(ModuleState moduleState, ModuleStateQueries stateQueries, ILogger logger)
+    {
+        var conflictingModule = stateQueries.FindModuleWithLockConflict(moduleState);
+        if (conflictingModule == null)
+        {
+            return;
+        }
+
+        var conflictingKeys = string.Join(", ",
+            moduleState.RequiredLockKeys.Intersect(conflictingModule.RequiredLockKeys));
+
+        logger.LogDebug(
+            "Module {ModuleName} blocked by lock conflict with {ConflictingModule} on keys: {Keys}",
+            MarkupFormatter.FormatModuleName(moduleState.Module.GetType().Name),
+            MarkupFormatter.FormatModuleName(conflictingModule.Module.GetType().Name),
+            conflictingKeys);
+    }
+}

src/ModularPipelines/Engine/Constraints/SequentialExecutionConstraint.cs

@@ -0,0 +1,49 @@
+using Microsoft.Extensions.Logging;
+using ModularPipelines.Helpers;
+using ModularPipelines.Logging;
+
+namespace ModularPipelines.Engine.Constraints;
+
+/// <summary>
+/// Enforces sequential execution constraints (NotInParallel attribute without keys)
+/// </summary>
+internal class SequentialExecutionConstraint : IModuleConstraint
+{
+    public bool CanExecute(ModuleState moduleState, ModuleStateQueries stateQueries)
+    {
+        // Check if blocked by another sequential module
+        var blockingModule = stateQueries.FindBlockingSequentialModule(moduleState);
+        if (blockingModule != null)
+        {
+            return false;
+        }
+
+        // Check if this is a sequential module blocked by others
+        if (moduleState.RequiresSequentialExecution)
+        {
+            return !stateQueries.AreOtherModulesActive(moduleState);
+        }
+
+        return true;
+    }
+
+    public void LogViolation(ModuleState moduleState, ModuleStateQueries stateQueries, ILogger logger)
+    {
+        var blockingModule = stateQueries.FindBlockingSequentialModule(moduleState);
+        if (blockingModule != null)
+        {
+            logger.LogDebug(
+                "Module {ModuleName} blocked by sequential module {SequentialModule}",
+                MarkupFormatter.FormatModuleName(moduleState.Module.GetType().Name),
+                MarkupFormatter.FormatModuleName(blockingModule.Module.GetType().Name));
+            return;
+        }
+
+        if (moduleState.RequiresSequentialExecution)
+        {
+            logger.LogDebug(
+                "Sequential module {ModuleName} blocked - other modules still running/queued",
+                MarkupFormatter.FormatModuleName(moduleState.Module.GetType().Name));
+        }
+    }
+}