[codex] Order Quartz lifecycle initializers#143
Conversation
Greptile SummaryThis PR fixes a race condition where the Quartz scheduler could start before provider-specific EF Core migrations ran, by leveraging the new lifecycle-ordering support in CShells
Confidence Score: 3/5The lifecycle-ordering logic is correct, but the entire solution is now pinned to a pre-release CShells package that may not restore cleanly on all CI or developer machines. The race-condition fix itself is well-structured and consistent across all four providers. The concern is the wholesale promotion of a pre-release dependency (0.0.21-preview.120) across the full solution — pre-release packages can be yanked, may require explicit NuGet feed configuration, and expose the entire project to API churn before the CShells lifecycle API stabilises. Directory.Packages.props — the pre-release CShells version bump affects every project in the solution, not just the Quartz modules.
|
| Filename | Overview |
|---|---|
| Directory.Packages.props | Bumps all CShells packages from stable 0.0.18 to pre-release 0.0.21-preview.120; introduces a dependency on an unstable NuGet feed for the entire solution |
| src/modules/scheduling/Elsa.Scheduling.Quartz/ShellFeatures/QuartzShellLifecycleHandler.cs | Adds [LifecycleOrder(LifecyclePhase.Start, 100)] attribute to pin scheduler startup to the Start phase, after provider migrations run in Prepare |
| src/modules/scheduling/Elsa.Scheduling.Quartz.EFCore.MySql/ShellFeatures/QuartzMySqlFeature.cs | Replaces manual IShellInitializer factory registration with AddShellInitializer(LifecyclePhase.Prepare, order: 100); DI injection of IDbContextFactory now happens implicitly |
| src/modules/scheduling/Elsa.Scheduling.Quartz.EFCore.PostgreSql/ShellFeatures/QuartzPostgreSqlFeature.cs | Same migration-handler registration change as the MySQL provider; pattern is consistent |
| src/modules/scheduling/Elsa.Scheduling.Quartz.EFCore.SqlServer/ShellFeatures/QuartzSqlServerFeature.cs | Same migration-handler registration change as the MySQL provider; pattern is consistent |
| src/modules/scheduling/Elsa.Scheduling.Quartz.EFCore.Sqlite/ShellFeatures/QuartzSqliteFeature.cs | Same migration-handler registration change as the MySQL provider; pattern is consistent |
| test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/ShellFeatures/QuartzFeatureTests.cs | Adds reflection-based test asserting QuartzShellLifecycleHandler carries [LifecycleOrder(Start, 100)]; does not add equivalent coverage for the EF migration handlers' Prepare-phase registration |
Sequence Diagram
sequenceDiagram
participant Shell as Shell Activation
participant Prepare as Prepare Phase
participant EF as EfCoreMigrationHandler (order: 100)
participant Start as Start Phase
participant Quartz as QuartzShellLifecycleHandler (order: 100)
Shell->>Prepare: Run Prepare initializers
Prepare->>EF: InitializeAsync()
EF->>EF: db.Database.MigrateAsync()
EF-->>Prepare: Done
Prepare-->>Shell: Prepare complete
Shell->>Start: Run Start initializers
Start->>Quartz: InitializeAsync()
Quartz->>Quartz: schedulerFactory.GetScheduler()
Quartz->>Quartz: scheduler.Start()
Quartz-->>Start: Done
Start-->>Shell: Shell active
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
Directory.Packages.props:144-150
**Pre-release package pinned across the solution**
All seven CShells packages are now pegged to `0.0.21-preview.120`, a pre-release version. Pre-release packages on NuGet are excluded from automatic `dotnet restore` on machines that don't explicitly opt into preview feeds, and they can be yanked or superseded without notice. This affects every consumer of this repo, not just the Quartz modules. Is there a stable release of this lifecycle-ordering feature planned, or is merging on the preview intentional until the API is finalized?
### Issue 2 of 2
test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/ShellFeatures/QuartzFeatureTests.cs:30-38
**EF migration handlers' Prepare-phase registration is untested**
The new test confirms that `QuartzShellLifecycleHandler` carries the `[LifecycleOrder(Start, 100)]` attribute, which is the consumer side of the ordering guarantee. The provider side — that each `EfCoreMigrationHandler` is registered under `LifecyclePhase.Prepare` with `order: 100` — has no equivalent coverage. If a future change accidentally reverts those registrations to the old unordered form (or uses the wrong phase), the race condition the PR is fixing would silently return without any failing test.
Reviews (1): Last reviewed commit: "Order Quartz lifecycle initializers" | Re-trigger Greptile
There was a problem hiding this comment.
Pull request overview
This PR updates the Quartz CShells lifecycle integration to ensure provider-specific EF Core migrations run before the Quartz scheduler starts, by leveraging CShells’ new lifecycle initializer ordering support.
Changes:
- Bump CShells package references to a preview version that supports lifecycle initializer ordering.
- Register Quartz EF Core migration initializers in the
Preparelifecycle phase with an explicit order. - Mark the Quartz scheduler lifecycle handler to run in the
Startlifecycle phase and add a unit test asserting the lifecycle metadata.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/ShellFeatures/QuartzFeatureTests.cs | Adds a unit test validating lifecycle ordering metadata for the Quartz scheduler handler. |
| src/modules/scheduling/Elsa.Scheduling.Quartz/ShellFeatures/QuartzShellLifecycleHandler.cs | Assigns an explicit lifecycle phase/order to ensure scheduler startup happens during Start. |
| src/modules/scheduling/Elsa.Scheduling.Quartz.EFCore.SqlServer/ShellFeatures/QuartzSqlServerFeature.cs | Registers EF Core migrations as a shell initializer in the Prepare phase. |
| src/modules/scheduling/Elsa.Scheduling.Quartz.EFCore.Sqlite/ShellFeatures/QuartzSqliteFeature.cs | Registers EF Core migrations as a shell initializer in the Prepare phase. |
| src/modules/scheduling/Elsa.Scheduling.Quartz.EFCore.PostgreSql/ShellFeatures/QuartzPostgreSqlFeature.cs | Registers EF Core migrations as a shell initializer in the Prepare phase. |
| src/modules/scheduling/Elsa.Scheduling.Quartz.EFCore.MySql/ShellFeatures/QuartzMySqlFeature.cs | Registers EF Core migrations as a shell initializer in the Prepare phase. |
| Directory.Packages.props | Updates CShells package versions to the lifecycle-ordering preview. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Why
Quartz scheduler startup could run before the provider-specific EF Core migration initializer, because feature dependencies still sort provider features after the base Quartz feature. CShells now separates feature dependency ordering from lifecycle initializer ordering, so providers can prepare storage before the runtime starts.
Validation
dotnet restore test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Elsa.Scheduling.Quartz.UnitTests.csproj --configfile /Users/sipke/.nuget/NuGet/NuGet.Config && dotnet test test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Elsa.Scheduling.Quartz.UnitTests.csproj --no-restoredotnet restore src/modules/scheduling/Elsa.Scheduling.Quartz.EFCore.PostgreSql/Elsa.Scheduling.Quartz.EFCore.PostgreSql.csproj --configfile /Users/sipke/.nuget/NuGet/NuGet.Config && dotnet build src/modules/scheduling/Elsa.Scheduling.Quartz.EFCore.PostgreSql/Elsa.Scheduling.Quartz.EFCore.PostgreSql.csproj --no-restoredotnet restore src/modules/scheduling/Elsa.Scheduling.Quartz.EFCore.SqlServer/Elsa.Scheduling.Quartz.EFCore.SqlServer.csproj --configfile /Users/sipke/.nuget/NuGet/NuGet.Config && dotnet build src/modules/scheduling/Elsa.Scheduling.Quartz.EFCore.SqlServer/Elsa.Scheduling.Quartz.EFCore.SqlServer.csproj --no-restoredotnet restore src/modules/scheduling/Elsa.Scheduling.Quartz.EFCore.Sqlite/Elsa.Scheduling.Quartz.EFCore.Sqlite.csproj --configfile /Users/sipke/.nuget/NuGet/NuGet.Config && dotnet build src/modules/scheduling/Elsa.Scheduling.Quartz.EFCore.Sqlite/Elsa.Scheduling.Quartz.EFCore.Sqlite.csproj --no-restoredotnet restore src/modules/scheduling/Elsa.Scheduling.Quartz.EFCore.MySql/Elsa.Scheduling.Quartz.EFCore.MySql.csproj --configfile /Users/sipke/.nuget/NuGet/NuGet.Config && dotnet build src/modules/scheduling/Elsa.Scheduling.Quartz.EFCore.MySql/Elsa.Scheduling.Quartz.EFCore.MySql.csproj --no-restoreWarnings observed are pre-existing: NU1903 for Snappier and an obsolete WorkflowGraphNotFoundException usage.