Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Feb 11, 2026

Addresses review feedback from #7279 to fix runtime DI failures and improve shell feature system integrity.

Feature Discovery

Added missing [ShellFeature] attributes to 7 features (Mediator, Multitenancy, SystemClock, DefaultFormatters, StringCompression, CommitStrategies, Expressions). Without these attributes, CShells cannot resolve DependsOn references, causing runtime failures.

Duplicate Registration Fixes

  • Caching: Removed caching service registrations from WorkflowManagementFeature and WorkflowRuntimeFeature — moved exclusively to CachingWorkflowDefinitionsFeature and CachingWorkflowRuntimeFeature to prevent double-decoration
  • Recurring tasks: Removed conflicting TriggerBookmarkQueueRecurringTask schedule (10s vs 1m)
  • Runtime services: Removed duplicate WorkflowResumer and BookmarkQueueWorker registrations
  • JavaScript: Removed duplicate ITypeDefinitionService registration
  • Flowchart: Removed registrations from WorkflowsFeature (already in FlowchartFeature)

Security & Compatibility

  • Removed hard-coded signing key from IdentityFeature — must be configured by application to prevent token forgery
  • Fixed null-safety in BlobStorageFeature.GetDefaultWorkflowsDirectory using AppContext.BaseDirectory fallback
  • Registered IInstalledFeatureProvider in legacy module system for API endpoint compatibility
  • Fixed XML doc mismatch in PersistenceShellFeatureBase (Scoped vs Singleton)

Cleanup

Removed commented-out CShells.Abstractions package references from Elsa.Common and Elsa.Workflows.Core projects.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

…es, and improve security

Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com>
Copilot AI changed the title [WIP] Integrate CShells-based modular shell features Fix shell feature discovery and remove duplicate service registrations Feb 11, 2026
Copilot AI requested a review from sfmskywalker February 11, 2026 19:51
@sfmskywalker sfmskywalker marked this pull request as ready for review February 11, 2026 21:06
@sfmskywalker sfmskywalker merged commit eaeb739 into enh/shells Feb 11, 2026
3 checks passed
@sfmskywalker sfmskywalker deleted the copilot/sub-pr-7279 branch February 11, 2026 21:08
@greptile-apps
Copy link

greptile-apps bot commented Feb 11, 2026

Greptile Overview

Greptile Summary

This PR addresses critical runtime DI failures and improves the shell feature system integrity by adding missing [ShellFeature] attributes and removing duplicate service registrations.

Key Changes:

  • Added [ShellFeature] attributes to 7 features (Mediator, Multitenancy, SystemClock, DefaultFormatters, StringCompression, CommitStrategies, Expressions) to enable CShells dependency resolution via DependsOn references
  • Removed duplicate caching service registrations from WorkflowManagementFeature and WorkflowRuntimeFeature - these are now exclusively handled by CachingWorkflowDefinitionsFeature and CachingWorkflowRuntimeFeature
  • Fixed conflicting TriggerBookmarkQueueRecurringTask schedule (removed 10-second schedule, keeping 1-minute schedule)
  • Removed duplicate WorkflowResumer and BookmarkQueueWorker registrations in WorkflowRuntimeFeature
  • Removed duplicate ITypeDefinitionService registration from JavaScriptFeature
  • Moved Flowchart service registrations from WorkflowsFeature to dedicated FlowchartFeature
  • Removed hard-coded signing key from IdentityFeature - applications must now configure this explicitly for security
  • Fixed null-safety in BlobStorageFeature.GetDefaultWorkflowsDirectory using AppContext.BaseDirectory fallback
  • Registered IInstalledFeatureProvider in legacy module system for API endpoint compatibility
  • Fixed XML doc mismatch in PersistenceShellFeatureBase
  • Cleaned up commented-out package references

The changes are well-structured and follow proper separation of concerns by moving duplicate registrations to dedicated feature modules.

Confidence Score: 4/5

  • This PR is generally safe to merge with one important consideration regarding the signing key change
  • The changes are well-focused on fixing DI issues and reducing duplication. Most changes are straightforward attribute additions and service registration removals. The main concern is the IdentityFeature signing key removal, which is a breaking change that requires applications to explicitly configure the signing key - this improves security but may break existing deployments if not documented properly
  • Pay close attention to src/modules/Elsa.Identity/ShellFeatures/IdentityFeature.cs - the signing key removal is a breaking change that requires migration steps for existing applications

Important Files Changed

Filename Overview
src/common/Elsa.Features/Implementations/Module.cs Added IInstalledFeatureProvider registration for API endpoint compatibility with legacy module system
src/modules/Elsa.Identity/ShellFeatures/IdentityFeature.cs Removed hard-coded signing key default value; applications must now configure SigningKey explicitly to prevent security issues
src/modules/Elsa.WorkflowProviders.BlobStorage/ShellFeatures/BlobStorageFeature.cs Fixed null-safety: now uses AppContext.BaseDirectory fallback when Assembly.GetEntryAssembly() returns null
src/modules/Elsa.Workflows.Management/ShellFeatures/WorkflowManagementFeature.cs Removed caching service registrations and decorators (moved to CachingWorkflowDefinitionsFeature)
src/modules/Elsa.Workflows.Runtime/ShellFeatures/WorkflowRuntimeFeature.cs Removed conflicting recurring task schedule, duplicate WorkflowResumer and BookmarkQueueWorker registrations, and caching decorators (moved to CachingWorkflowRuntimeFeature)

Sequence Diagram

sequenceDiagram
    participant App as Application Startup
    participant Module as Module System
    participant WM as WorkflowManagementFeature
    participant WR as WorkflowRuntimeFeature
    participant CWD as CachingWorkflowDefinitionsFeature
    participant CWR as CachingWorkflowRuntimeFeature
    participant Shell as Shell Features (7 new attributes)

    Note over App,Shell: Before PR: Duplicate Registrations
    App->>Module: Configure Features
    Module->>WM: ConfigureServices()
    WM->>WM: Register IWorkflowDefinitionStore
    WM->>WM: Register Caching Decorators ❌
    Module->>WR: ConfigureServices()
    WR->>WR: Register WorkflowResumer (twice) ❌
    WR->>WR: Register BookmarkQueueWorker (twice) ❌
    WR->>WR: Configure TriggerBookmarkQueue (10s) ❌
    WR->>WR: Register TriggerBookmarkQueue (1m) ❌
    WR->>WR: Register Caching Decorators ❌

    Note over App,Shell: After PR: Clean Separation
    App->>Module: Configure Features
    Module->>Shell: Resolve DependsOn references ✓
    Shell->>Shell: ShellFeature attributes enable resolution
    Module->>WM: ConfigureServices()
    WM->>WM: Register IWorkflowDefinitionStore only ✓
    Module->>CWD: ConfigureServices()
    CWD->>CWD: Register Caching Decorators ✓
    Module->>WR: ConfigureServices()
    WR->>WR: Register WorkflowResumer (once) ✓
    WR->>WR: Register BookmarkQueueWorker (once) ✓
    WR->>WR: Register TriggerBookmarkQueue (1m) ✓
    Module->>CWR: ConfigureServices()
    CWR->>CWR: Register Caching Decorators ✓
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

17 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment on lines +59 to +60
// Configure options - Note: SigningKey must be configured by the application for security
services.Configure<IdentityTokenOptions>(_ => { });
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Breaking change - existing applications using IdentityFeature will need to explicitly configure the SigningKey in their startup code, otherwise token generation/validation will fail at runtime. Consider documenting migration steps.

sfmskywalker added a commit that referenced this pull request Feb 12, 2026
* Add CShells package references and integrate shell features into the application

* Annotate shell features with `[ShellFeature]` attribute and update `TempElsaFeature` to use `ConfigureElsa`.

* Revert "Annotate shell features with `[ShellFeature]` attribute and update `TempElsaFeature` to use `ConfigureElsa`."

This reverts commit e8a875e.

* Introduce `Elsa.ModularServer.Web` with a minimal API, restructure shell feature configuration, and remove obsolete `CShells` dependency

* Update `CShells` package references, add Fody weaver, and configure new CShells package sources in `NuGet.Config`.

* Adds CShells integration to Elsa

Integrates CShells to enhance modularity and extensibility.

- Adds CShells related projects to the solution.
- Updates NuGet configuration to include CShells preview feed.
- Creates initial app settings for CShells configuration.
- Adds CShells.AspNetCore project reference.
- Implements CShells extensions in the program file.
- Creates shell feature classes in Elsa.Common.
- Creates shell feature classes in Elsa.Expressions.
- Creates shell feature classes in Elsa.Workflows.Core.
- Creates shell feature classes in Elsa.Workflows.Management.
- Creates shell feature classes in Elsa.Workflows.Runtime.
- Creates shell feature classes in Elsa module.

* Refactor CShells: enhance pipeline configuration and features

Consolidated updates to CShells including a new `ResolverPipelineBuilder` for customizable resolver strategy pipelines. Improved assembly scanning, error handling in web routing, and streamlined shell feature dependencies for better clarity and functionality.

* Add feature registration system and FastEndpoints integration

Introduced a feature registration infrastructure with `IInstalledFeatureProvider` and related implementations. Added shell-based feature configurations such as caching, SAS tokens, workflows management, and a FastEndpoints integration module to support dynamic API registration.

* Refactor shell routing and enhance global route handling

Refactored `ShellEndpointRouteBuilder` to simplify initialization and support a combined shell/global route prefix. Enhanced `ShellEndpointRegistrationHandler` to include global route prefix logic and improved feature discovery using pre-resolved descriptors. Updated `Program.cs` for consistent middleware setup.

* Refactor feature endpoints to use `IInstalledFeatureProvider` for improved dependency management and simplified implementation

* Add display names, descriptions, and dependency enhancements to shell features

Standardized `ShellFeature` attributes across `ElsaFeature`, `WorkflowRuntimeFeature`, and `WorkflowManagementFeature` by adding display names, descriptions, and improving dependency declarations. Updated `ElsaFeature` to register `IInstalledFeatureProvider` for feature bridging.

* Add FastEndpoints references and update package versions

Added project references to CShells.FastEndpoints and related projects in multiple csproj files. Updated FastEndpoints package versions in `Directory.Packages.props` for compatibility with .NET 8/9/10. Removed obsolete folder references from Elsa.Caching.csproj and refined the namespace in CShells.AspNetCore.Abstractions.

* Add Identity and DefaultAuthentication features to appsettings.json configuration

* Add project references for Elsa.Identity and CShells.FastEndpoints.Abstractions

* Add `Identity` and `DefaultAuthentication` shell features with enhanced authentication and authorization support

* Set default signing key in `IdentityTokenOptions` for identity configuration

* Add service exclusion infrastructure for shell-specific contexts

Introduce `IShellServiceExclusionProvider` and `IShellServiceExclusionRegistry` to manage excluded service types per-shell. Implement ASP.NET Core-specific providers for authentication and authorization to enable shell-specific configurations. Refactor `DefaultShellHost` to use the new exclusion registry for service inheritance filtering.

* Refactor CShells authentication and authorization APIs

Renamed and unified methods for shell authentication and authorization and added a new combined method `WithAuthenticationAndAuthorization`. Enhanced `AddShells` to automatically register a default configuration provider if none is specified. Updated usage in Elsa.ModularServer to utilize the new API.

* Add Elsa-specific FastEndpoints configurator and feature

Introduce `ElsaFastEndpointsConfigurator` to customize FastEndpoints serialization and value parsing for Elsa workflows. Register this functionality through the new `ElsaFastEndpointsFeature`, which integrates with the shell's dependency injection system using an `IFastEndpointsConfigurator` interface.

* Update Workflow API feature dependency to `ElsaFastEndpoints`

* Pass `cancellationToken` to `ReadToEndAsync` in `PostEndpoint` for improved request handling.

* Add project references for CShells.AspNetCore and CShells.FastEndpoints.Abstractions

* Update CShells package versions to `0.0.6-preview.30` and add `CShells.FastEndpoints.Abstractions`

* Configures shell routing and features

Enables path routing for shells to allow proper routing within each shell.

Configures the ElsaFastEndpoints feature to depend on the FastEndpoints feature.
This ensures that FastEndpoints is properly configured before Elsa's FastEndpoints configurations are applied.

Registers activity types within the WorkflowManagementFeature.
This ensures activities are available for workflow construction and execution.

* Add shell lifecycle management and notification handlers

Introduced interfaces and handlers for shell activation (`IShellActivatedHandler`) and deactivation (`IShellDeactivatingHandler`) to manage shell lifecycles. Added `ShellStartupHostedService` to coordinate shell activation on startup and deactivation on shutdown. Updated notification system to support new shell lifecycle events and renamed existing notification records for consistency.

* Introduces EF Core persistence layer

Adds base classes and implementations for EF Core persistence, including database provider configuration and shell feature integration.

This change introduces a generic approach to configuring EF Core persistence for various Elsa modules, promoting code reuse and simplifying the process of supporting different database providers.

It includes:

- Base classes for database provider configurators and shell features.
- Implementations for Sqlite, SQL Server, MySql, PostgreSql, and Oracle.
- Shell features for Alterations, Identity, Labels, Management (Workflow Definitions and Instances), Runtime, and Tenants modules.

* Add comprehensive feature configuration validation system

Introduce a feature configuration system with support for binding, auto-configuration, and validation using DataAnnotations, FluentValidation, and composite patterns. Includes new validators, binding logic, and extensions to simplify configuration tasks while ensuring robustness and flexibility.

* Add persistence shell features for MySql, Oracle, PostgreSql, and Sqlite

Introduced new shell features to configure MySql, Oracle, PostgreSql, and Sqlite persistence for workflow definitions and runtime data. Updated `appsettings.json` to replace individual Sqlite features with a unified `SqliteWorkflowPersistence`. Made minor code cleanup in `FastEndpointsFeature`.

* Configure shell features for persistence

Added `IServiceCollection` configuration for MySql, Oracle, PostgreSql, and Sqlite shell features to set up persistence services.

* Remove DatabaseProviderConfigurators and refactor persistence shell features

Deleted DatabaseProviderConfigurator classes and restructured persistence shell features by integrating direct configuration logic for MySql, Oracle, PostgreSql, Sqlite, and SqlServer. Simplified configuration by inheriting from abstract shell feature base classes and removed redundant code.

* Correct IWorkflowDefinitionPublisher registration to use WorkflowDefinitionPublisher implementation

* Add resilience feature and scoped services configuration for Sqlite persistence

- Integrated `Microsoft.Extensions.DependencyInjection` to shell features for Sqlite persistence.
- Updated `appsettings.json` and project references to include a new 'Resilience' feature.
- Changed `ICommitStateHandler` service registration in `WorkflowRuntimeFeature` to use an implementation.

* Add `ResilienceShellFeature` for configuring resilience strategies

- Implemented new `ResilienceShellFeature` class to manage services related to resilience features.
- Added scoped and singleton service registrations for resilience strategies, exception detection, and activity invocation.
- Configured expression options for resilience handling in workflows.

* Add new shell features: Alterations, Blob Storage, Caching, Clustering, CSharp, Distributed Runtime, ElsaScript, Flowchart, HTTP, JavaScript, Key-Value, and Labels

* Switch project references to package references for CShells libraries and update to version 0.0.7.

* Potential fix for pull request finding 'Call to 'System.IO.Path.Combine' may silently drop its earlier arguments'

Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>

* Fix shell feature discovery and remove duplicate service registrations (#7285)

* Initial plan

* Address PR review comments: Add ShellFeature attributes, fix duplicates, and improve security

Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com>

* Update dependencies and fix scoped services registration in `WorkflowRuntimeFeature`

* Fix null reference and typo in shell feature provider (#7286)

* Initial plan

* Fix null StartupType guard in Find() and typo in variable descriptor

Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com>

---------

Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: sfmskywalker <938393+sfmskywalker@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants