VPR-54 feat(scheduler): Hangfire-backed scheduler#182
Conversation
Bundle ReportChanges will increase total bundle size by 215 bytes (0.01%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: viper-frontend-esmAssets Changed:
Files in
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #182 +/- ##
==========================================
+ Coverage 42.95% 43.01% +0.06%
==========================================
Files 876 888 +12
Lines 51454 51790 +336
Branches 4802 4834 +32
==========================================
+ Hits 22101 22279 +178
- Misses 28829 28982 +153
- Partials 524 529 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughThis PR implements a complete Hangfire-based background job scheduler for VIPER. It adds job discovery via reflection and ChangesHangfire Scheduler Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 23
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/Scheduler/JobsControllerTests.cs`:
- Around line 223-235: The test
ResumeJob_Returns404WithMarkerNotFoundCodeWhenMissing currently only asserts the
response type; update it to also assert the NotFound payload contains the
marker_not_found code by casting result.Result to NotFoundObjectResult and
verifying the response value (from _sut.ResumeJob with
JobsController.ResumeRequest) has a code or error field equal to
"marker_not_found" (keeping the existing SchedulerJobNotFoundException setup);
ensure the assertion checks the same shape the controller returns so the test
fails if the marker code is missing.
In `@test/Viper.test.csproj`:
- Line 37: Update the PackageReference for Hangfire.Core in the test project
from Version="1.8.21" to Version="1.8.23" so it matches the production/web
project; locate the PackageReference element for "Hangfire.Core" in
Viper.test.csproj and change the Version attribute to "1.8.23", then
restore/update NuGet packages (dotnet restore or equivalent) to ensure the
lock/assets are regenerated.
In `@web/appsettings.json`:
- Around line 39-43: Remove the standalone // comment block between top-level
JSON properties that breaks Biome: delete the three lines of // commentary so
the "Hangfire" object is immediately contiguous with other properties, and if
documentation is needed move it to the existing .env.local.example or convert it
into an inline trailing comment on the "Enabled" property (e.g., a single-line
comment after "Enabled") rather than a separate JSON line; target the "Hangfire"
object and its "Enabled" property when making this change.
In `@web/Areas/RAPS/Services/RAPSAuditService.cs`:
- Around line 256-261: The ModBy fallback in AuditRoleMemberChange currently
only handles null; change the logic so empty or whitespace modBy values are
treated like null by using string.IsNullOrWhiteSpace(modBy) when assigning
TblLog.ModBy, falling back to UserHelper.GetCurrentUser()?.LoginId (optionally
trimmed) so audit entries never record blank actors; update the assignment in
AuditRoleMemberChange (and any similar assignments for TblLog.ModBy in this
class) to use this check.
In `@web/Areas/RAPS/Services/RoleViews.cs`:
- Around line 42-54: Both UpdateRoles and UpdateRole duplicate the
actor-resolution logic; create a single helper like ResolveActor(string? modBy)
=> string.IsNullOrEmpty(modBy) ? DefaultModBy : modBy and use it from both
UpdateRoles and UpdateRole so the fallback rule is centralized; update both
methods to call ResolveActor(modBy) (or make it static/private as appropriate)
and remove the duplicated conditional logic in each method.
In `@web/Areas/Scheduler/Controllers/JobsController.cs`:
- Around line 89-92: In JobsController catch blocks that handle
SchedulerConcurrencyException (e.g., in PauseJob and ResumeJob), add a warning
log call before returning Conflict to surface the failure to telemetry/logs; log
contextual details such as jobId and user (or requestor) and the exception
message so operators can correlate the 409 response with the warning; use the
controller's ILogger instance (e.g., _logger) and include a clear message like
"Concurrency conflict while attempting to pause/resume job {jobId} for user
{userId}" followed by the exception/info, then return Conflict(new { error =
ConcurrencyErrorCode }).
- Line 19: The ApiController base class lacks the [ApiController] attribute so
derived controllers like JobsController won't get ApiController behavior; open
the ApiController class (the declaration "public class ApiController :
ControllerBase") and add the [ApiController] attribute directly above that class
declaration so the attribute is applied to the base class used by
JobsController.
In `@web/Areas/Scheduler/Models/Entities/SchedulerJobState.cs`:
- Around line 28-29: The XML doc for the PausedAt property incorrectly says
"UTC" while project guidelines use local time for DB timestamps; update the
summary on the SchedulerJobState.PausedAt property to indicate it is a local
timestamp (and ensure code that sets it uses DateTime.Now/DateTimeKind.Local or
DateTimeOffset local semantics). If there is a true spec dependency requiring
UTC (e.g., reconciliation with Hangfire), instead leave UTC and add a clear
inline comment on SchedulerJobState.PausedAt explaining the exception and why
UtcNow is required.
In `@web/Areas/Scheduler/Models/ScheduledJobContext.cs`:
- Around line 11-15: The ScheduledJobContext constructor currently accepts a
null/blank modBy which breaks downstream audit invariants; update the
ScheduledJobContext(TriggerSource triggerSource, string modBy) constructor to
validate modBy (e.g., check for null or whitespace), throw ArgumentNullException
or ArgumentException with a clear message when invalid, and only assign ModBy
when valid (optionally trim the value before assignment) so invalid contexts
cannot be created.
In `@web/Areas/Scheduler/README.md`:
- Around line 172-174: The fenced code block using backticks containing the
scheduler reconciler message lacks a language specifier; update the fence
starting marker (```) to include a language tag such as "text" (e.g., change ```
to ```text) so syntax highlighters/renderers treat the content as plain
text—modify the fenced block around the line that begins "Scheduler reconciler
pass: split-brain healed=N, system markers deleted=N, paused ok=N, active ok=N,
lost registrations healed=N, markers=N, registrations=N".
- Around line 23-41: The README example constructor for
RapsRoleRefreshScheduledJob is missing the ILogger dependency; update the
snippet to include an ILogger<RapsRoleRefreshScheduledJob> parameter in the
constructor signature, add a private readonly field (e.g., _logger) to the class
and assign the injected logger in the constructor (keeping the existing
RAPSContext _rapsContext assignment), and ensure the example includes the using
for Microsoft.Extensions.Logging so the DI signature matches the real
implementation and won't fail at runtime when constructing
RapsRoleRefreshScheduledJob.
In `@web/Areas/Scheduler/Services/ScheduledJobDiscovery.cs`:
- Around line 111-125: The ResolveTimeZone method currently swallows
TimeZoneNotFoundException and InvalidTimeZoneException and silently returns
TimeZoneInfo.Utc; update ResolveTimeZone in ScheduledJobDiscovery to log a
warning (including the supplied id and exception message/stack) when a lookup
fails so ops can detect misconfigured timezones, then continue to return
TimeZoneInfo.Utc; use the existing logger on the class (or inject
ILogger<ScheduledJobDiscovery> if none exists) and include the TimeZoneId and
exception details in the warning log entry.
In `@web/Areas/Scheduler/Services/SchedulerJobReconciler.cs`:
- Around line 71-92: ResolvePacific currently returns TimeZoneInfo.Utc silently
when neither "Pacific Standard Time" nor "America/Los_Angeles" resolve; change
ResolvePacific to log a warning right before returning UTC so operators see the
fallback. Update ResolvePacific to accept or reference the reconciler's ILogger
(or use the existing logger field) and call logger.LogWarning with a message
that includes the attempted ids and that UTC is being used as a fallback (e.g.,
"Neither 'Pacific Standard Time' nor 'America/Los_Angeles' resolved; falling
back to UTC"). Preserve the existing exception handling and still return
TimeZoneInfo.Utc after logging.
- Around line 113-126: StartAsync currently awaits
_reconciler.RunOnceAsync(cancellationToken) which blocks host startup; change it
to launch the reconciler pass in the background instead: kick off a non-blocking
Task (e.g., use Task.Run or Task.Factory.StartNew with the cancellationToken)
that calls _reconciler.RunOnceAsync and catches/logs exceptions via
_logger.LogError(ex, "...") so failures are recorded but do not delay startup;
ensure the fire-and-forget Task is started (optionally await Task.Yield() first)
and that the cancellationToken is honored inside the background delegate.
In `@web/Areas/Scheduler/Services/SchedulerJobsService.cs`:
- Line 134: Change the timestamp used for DB audit fields from UTC to local
time: replace uses of DateTime.UtcNow assigned to the PausedAt property in
SchedulerJobsService (and the other occurrence noted) with DateTime.Now so the
persisted value uses DateTimeKind.Local; update both assignment sites where
PausedAt is set to ensure DB-stored timestamps follow the project's guideline to
default to local time.
- Around line 324-338: Merge the duplicate methods ResolveDeclaredTimeZone and
ResolveTimeZone by keeping one implementation (rename to ResolveTimeZoneSafe or
keep ResolveTimeZone) that returns TimeZoneInfo.FindSystemTimeZoneById(id)
inside try/catch and falls back to TimeZoneInfo.Utc on TimeZoneNotFoundException
or InvalidTimeZoneException, then remove the other method and update all
call-sites that reference ResolveDeclaredTimeZone or ResolveTimeZone to call the
single surviving method name.
- Around line 55-62: Replace the manual foreach/filter/add with a LINQ pipeline:
filter the markers using .Where(m => !seenIds.Contains(m.RecurringJobId)) and
then map with .Select(m => BuildDtoFromMarker(m)), and add the resulting
sequence into results (use results.AddRange or equivalent); refer to the local
symbols markers, seenIds, BuildDtoFromMarker, and results to locate and
implement the change.
- Around line 160-175: The broad catch(Exception) around
_recurringJobManager.RemoveIfExists(id) must be replaced with explicit catches
for the known Hangfire/storage failure types (e.g.,
DistributedLockTimeoutException, BackgroundJobClientException and any observed
SqlException) so unexpected exceptions can propagate; catch those specific
exception types, set deregistrationPending = true and call _logger.LogError(...)
using LogSanitizer.SanitizeId(id) in each specific catch, and remove the generic
catch so service-level logic does not swallow unrelated failures.
- Line 122: hangfireJob.Job can be null so calling
SerializeJobPayload(hangfireJob.Job) causes an NRE when SerializeJobPayload
dereferences job.Method; change the call site in SchedulerJobsService to check
hangfireJob.Job for null and only call SerializeJobPayload when non-null (e.g.,
set payload to null or an appropriate placeholder when hangfireJob.Job is null),
or update SerializeJobPayload to accept a nullable parameter and safely handle a
null job (avoid dereferencing job.Method). Ensure you reference
SerializeJobPayload and hangfireJob.Job when applying the fix.
In `@web/Classes/Scheduler/HangfireDashboardAuthorizationFilter.cs`:
- Around line 41-43: Guard against a null/empty identity name before calling
userHelper.GetByLoginId: check user?.Identity?.Name (or equivalent) and if it's
null or empty return false early; then call userHelper.GetByLoginId(aaudContext,
name) and proceed to evaluate aaudUser != null &&
userHelper.HasPermission(rapsContext, aaudUser, SchedulerPermission). This
prevents passing a null name into GetByLoginId and avoids incorrect/exceptional
behavior.
In `@web/Classes/Scheduler/HangfireExtensions.cs`:
- Around line 67-69: The scheduled-job discovery currently calls
ScheduledJobDiscovery.RegisterScheduledJobs(services, new[] {
Assembly.GetExecutingAssembly() }) which only scans the executing assembly;
change the call so discovery can see sibling/test assemblies—either replace
Assembly.GetExecutingAssembly() with a filtered
AppDomain.CurrentDomain.GetAssemblies() (e.g., filter by your project's
namespace prefix) or update the RegisterScheduledJobs API to accept an
Assembly[] parameter and pass in a merged list
(AppDomain.CurrentDomain.GetAssemblies().Where(...)). Ensure you reference
ScheduledJobDiscovery.RegisterScheduledJobs and the Assembly selection logic so
future assemblies with [ScheduledJob] are discoverable.
In `@web/Classes/Scheduler/SchedulerSchemaInitializer.cs`:
- Around line 52-58: The catch block in SchedulerSchemaInitializer that
currently reads "catch (Exception ex)" should be narrowed to catch SqlException
only: change the exception type in the catch to SqlException (e.g., catch
(SqlException ex)), keep the existing logger.Error(ex, "...") call, and add the
appropriate using/import for the SQL client library used in the project
(System.Data.SqlClient or Microsoft.Data.SqlClient) so the type resolves; do not
alter the logging text or behavior beyond changing the caught exception type.
In `@web/Viper.csproj`:
- Around line 52-53: Update the Hangfire package references in Viper.csproj:
change the Version for both PackageReference entries "Hangfire.AspNetCore" and
"Hangfire.SqlServer" from 1.8.21 to 1.8.23 to pull in fixes for the dashboard
CSS map and the endpoint pipeline InvalidOperationException; ensure the csproj
now uses Version="1.8.23" for both package references and run a restore/build to
verify no compatibility issues.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c549a843-f260-47eb-97ac-c0f6ff16a579
📒 Files selected for processing (43)
.env.local.exampletest/HealthChecks/HangfireHealthCheckTests.cstest/RAPS/RapsRoleRefreshScheduledJobTests.cstest/Scheduler/HangfireJobLoggingFilterTests.cstest/Scheduler/JobsControllerTests.cstest/Scheduler/ScheduledJobDiscoveryTests.cstest/Scheduler/ScheduledJobRunnerTests.cstest/Scheduler/SchedulerJobsServiceTests.cstest/Viper.test.csprojweb/Areas/RAPS/Jobs/RapsRoleRefreshScheduledJob.csweb/Areas/RAPS/Services/RAPSAuditService.csweb/Areas/RAPS/Services/RoleViews.csweb/Areas/Scheduler/Controllers/.gitkeepweb/Areas/Scheduler/Controllers/JobsController.csweb/Areas/Scheduler/Models/DTOs/Responses/PauseResumeResultDto.csweb/Areas/Scheduler/Models/DTOs/Responses/ReconcilerOutcomeDto.csweb/Areas/Scheduler/Models/DTOs/Responses/SchedulerJobDto.csweb/Areas/Scheduler/Models/Entities/SchedulerJobState.csweb/Areas/Scheduler/Models/ScheduledJobContext.csweb/Areas/Scheduler/Models/TriggerSource.csweb/Areas/Scheduler/README.mdweb/Areas/Scheduler/Services/.gitkeepweb/Areas/Scheduler/Services/IScheduledJob.csweb/Areas/Scheduler/Services/ISchedulerJobsService.csweb/Areas/Scheduler/Services/ScheduledJobAttribute.csweb/Areas/Scheduler/Services/ScheduledJobDiscovery.csweb/Areas/Scheduler/Services/ScheduledJobRegistry.csweb/Areas/Scheduler/Services/ScheduledJobRunner.csweb/Areas/Scheduler/Services/SchedulerExceptions.csweb/Areas/Scheduler/Services/SchedulerJobReconciler.csweb/Areas/Scheduler/Services/SchedulerJobsService.csweb/Classes/ForwardedHeadersExtensions.csweb/Classes/HealthChecks/HangfireHealthCheck.csweb/Classes/SQLContext/SchedulerContext.csweb/Classes/SQLContext/VIPERContext.csweb/Classes/Scheduler/HangfireDashboardAuthorizationFilter.csweb/Classes/Scheduler/HangfireExtensions.csweb/Classes/Scheduler/HangfireJobLoggingFilter.csweb/Classes/Scheduler/SchedulerSchemaInitializer.csweb/Program.csweb/Views/Shared/Components/MainNav/MainNav.csweb/Viper.csprojweb/appsettings.json
There was a problem hiding this comment.
Pull request overview
Introduces a new Hangfire-backed background scheduler subsystem (gated by Hangfire:Enabled) with secured dashboard access, operator pause/resume APIs, reconciliation logic, health checks, and an initial scheduled consumer for nightly RAPS role refresh.
Changes:
- Add Hangfire + SQL Server storage integration, with dashboard endpoint and per-job logging/telemetry.
- Implement scheduler domain model:
[ScheduledJob]discovery/registration, pause/resume marker table, and reconciler (startup + hourly). - Add scheduler admin API (
/api/scheduler/jobs) and first scheduled job (raps:role-refresh), threading explicit audit actor (modBy) through RAPS role updates.
Reviewed changes
Copilot reviewed 41 out of 43 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| web/Viper.csproj | Adds Hangfire package references and Scheduler area folders. |
| web/Views/Shared/Components/MainNav/MainNav.cs | Adds “Scheduler” nav link to the Hangfire dashboard and updates title mapping. |
| web/Program.cs | Wires forwarded-headers via extension and hooks Hangfire DI + middleware behind Hangfire:Enabled. |
| web/Classes/SQLContext/VIPERContext.cs | Adds scheduler model creation hook to EF context. |
| web/Classes/SQLContext/SchedulerContext.cs | Adds EF DbSet + mapping for [HangFire].[SchedulerJobState]. |
| web/Classes/Scheduler/SchedulerSchemaInitializer.cs | Adds idempotent DDL initializer for the scheduler marker table. |
| web/Classes/Scheduler/HangfireJobLoggingFilter.cs | Adds Hangfire server filter for structured logging scopes and sanitized job start/finish/error logs. |
| web/Classes/Scheduler/HangfireExtensions.cs | Adds Hangfire DI registration, dashboard mount, health check registration, and scheduled job registration bootstrap. |
| web/Classes/Scheduler/HangfireDashboardAuthorizationFilter.cs | Adds RAPS-permission authorization for the Hangfire dashboard. |
| web/Classes/HealthChecks/HangfireHealthCheck.cs | Adds Hangfire readiness health check (storage + server heartbeat freshness). |
| web/Classes/ForwardedHeadersExtensions.cs | Moves Cloudflare/F5 forwarded-headers configuration out of Program.cs. |
| web/Areas/Scheduler/Services/SchedulerJobsService.cs | Implements list/get/pause/resume/reconcile logic coordinating Hangfire + marker table. |
| web/Areas/Scheduler/Services/SchedulerJobReconciler.cs | Adds reconciler driver + recurring registration + startup hosted service pass. |
| web/Areas/Scheduler/Services/SchedulerExceptions.cs | Adds typed exceptions for controller translation (403/404/409). |
| web/Areas/Scheduler/Services/ScheduledJobRunner.cs | Adds DI-based dispatcher to execute scheduled jobs via a concrete target for Hangfire. |
| web/Areas/Scheduler/Services/ScheduledJobRegistry.cs | Adds metadata registry abstraction for declared scheduled jobs. |
| web/Areas/Scheduler/Services/ScheduledJobDiscovery.cs | Adds reflection-based discovery/validation/DI registration for [ScheduledJob] implementations. |
| web/Areas/Scheduler/Services/ScheduledJobAttribute.cs | Adds attribute contract for recurring job id/cron/timezone/system-flag. |
| web/Areas/Scheduler/Services/ISchedulerJobsService.cs | Defines scheduler operator service API + reserved id namespace constants. |
| web/Areas/Scheduler/Services/IScheduledJob.cs | Defines scheduled job execution contract with ScheduledJobContext. |
| web/Areas/Scheduler/Services/.gitkeep | Keeps the services folder tracked. |
| web/Areas/Scheduler/README.md | Adds scheduler onboarding/configuration and ops runbook documentation. |
| web/Areas/Scheduler/Models/TriggerSource.cs | Adds enum to distinguish scheduled vs manual runs. |
| web/Areas/Scheduler/Models/ScheduledJobContext.cs | Adds per-run context including trigger source and audit actor (ModBy). |
| web/Areas/Scheduler/Models/Entities/SchedulerJobState.cs | Adds marker entity representing paused job definition + rowversion. |
| web/Areas/Scheduler/Models/DTOs/Responses/SchedulerJobDto.cs | Adds operator-facing job DTO combining Hangfire + paused marker state. |
| web/Areas/Scheduler/Models/DTOs/Responses/ReconcilerOutcomeDto.cs | Adds reconciler outcome counters DTO. |
| web/Areas/Scheduler/Models/DTOs/Responses/PauseResumeResultDto.cs | Adds pause/resume result DTO with RowVersion and pending deregistration flag. |
| web/Areas/Scheduler/Controllers/JobsController.cs | Adds secured scheduler operator API endpoints (list/get/pause/resume). |
| web/Areas/Scheduler/Controllers/.gitkeep | Keeps the controllers folder tracked. |
| web/Areas/RAPS/Services/RoleViews.cs | Threads explicit modBy through role refresh path and preserves legacy default actor. |
| web/Areas/RAPS/Services/RAPSAuditService.cs | Allows explicit modBy override for audit rows (fallbacks to current CAS user). |
| web/Areas/RAPS/Jobs/RapsRoleRefreshScheduledJob.cs | Adds nightly scheduled job wrapping RAPS role refresh with scheduler audit actor. |
| web/appsettings.json | Adds Hangfire:Enabled (default false) configuration block. |
| test/Viper.test.csproj | Adds Hangfire.Core test dependency. |
| test/Scheduler/SchedulerJobsServiceTests.cs | Adds unit tests covering list/pause/resume/reconcile service behavior. |
| test/Scheduler/ScheduledJobRunnerTests.cs | Adds tests for runner job resolution and context stamping. |
| test/Scheduler/ScheduledJobDiscoveryTests.cs | Adds tests validating discovery and system-prefix invariants. |
| test/Scheduler/JobsControllerTests.cs | Adds controller tests for status codes and error payloads. |
| test/Scheduler/HangfireJobLoggingFilterTests.cs | Adds tests asserting logging, scopes, and error/completion behavior. |
| test/RAPS/RapsRoleRefreshScheduledJobTests.cs | Adds tests verifying scheduled job attribute metadata for RAPS job. |
| test/HealthChecks/HangfireHealthCheckTests.cs | Adds tests for healthy/degraded/unhealthy Hangfire health outcomes. |
| .env.local.example | Documents the optional local env var toggle for Hangfire enablement. |
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (1)
web/appsettings.json (1)
39-43:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winBiome still rejects
//comments inside the JSON object — same issue as before.Moving the comments inside
"Hangfire"instead of between top-level keys didn't fix the parse errors; Biome treats the whole file as strict JSON. Consolidate to a single trailing comment on"Enabled"(JSONC-style inline comments are fine in this config reader).🛠️ Proposed fix
"Hangfire": { - // Disabled by default; each environment opts in via Hangfire:Enabled. - // Hangfire and the marker table share ConnectionStrings:VIPER. - "Enabled": false + "Enabled": false // Disabled by default; enable per env. Shares ConnectionStrings:VIPER. }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/appsettings.json` around lines 39 - 43, The Hangfire section contains line-comment blocks that break the strict JSON parser; remove the standalone // comments between top-level keys and replace them with a single trailing inline comment on the "Enabled" property (inside the "Hangfire" object) using JSONC-style inline comment syntax so the config reader can parse it (update the "Hangfire" object that contains "Enabled" accordingly).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/Areas/RAPS/Jobs/RapsRoleRefreshScheduledJob.cs`:
- Around line 30-34: RunAsync currently accepts ct but doesn't thread it into
the role refresh, so update the signature of RoleViews.UpdateRoles to accept a
CancellationToken and call it from RunAsync passing ct; likewise add
CancellationToken parameters to RoleViews.UpdateRole, GetRoleMembers, and
GetViewMembers and propagate ct through their call chain. Inside those methods,
pass the token into all EF Core async calls (e.g., .ToListAsync(ct),
FirstOrDefaultAsync(ct), etc.) across the ~30 queries in GetViewMembers, and
change synchronous SaveChanges() usages in AddRoleMember and DeleteRoleMember to
async SaveChangesAsync(ct) and make those methods async (propagate the token up
to callers). Ensure all call sites are updated to await the new async methods
and forward the CancellationToken.
In `@web/Areas/Scheduler/README.md`:
- Around line 188-190: The README incorrectly references a non-existent
ConnectionStrings:Hangfire override; either remove the "(and `:Hangfire` if
overridden)" text from the README to accurately state that Hangfire wiring only
reads ConnectionStrings:VIPER, or implement the actual override by updating
HangfireExtensions.AddViperHangfire and UseViperHangfire so they check for
ConnectionStrings:Hangfire (falling back to ConnectionStrings:VIPER) and use
that value for the HangFire schema login; pick one approach and make the README
and the methods (AddViperHangfire / UseViperHangfire) consistent.
In `@web/Areas/Scheduler/Services/ScheduledJobRegistry.cs`:
- Around line 9-30: ScheduledJobMetadata is an immutable value holder and should
be converted to a positional record to remove constructor/property boilerplate
and gain structural equality; replace the class declaration and its explicit
constructor/properties with a positional record declaration for
ScheduledJobMetadata taking (Type jobType, string id, string cron, string
timeZoneId, bool isSystem) and update any references to use the record's
autogenerated properties (JobType, Id, Cron, TimeZoneId, IsSystem).
In `@web/Areas/Scheduler/Services/SchedulerJobsService.cs`:
- Around line 193-265: ResumeJobAsync currently calls
_recurringJobManager.AddOrUpdate(id, ...) before persisting the marker removal
and only catches DbUpdateConcurrencyException, so non-concurrency
SaveChangesAsync failures leave Hangfire registered while the API returns an
error. Update ResumeJobAsync to either (A) mirror PauseJobAsync by catching
broader persistence failures (catch DbUpdateException and/or TimeoutException)
and log the error while returning PauseResumeResultDto with
DeregistrationPending = true, or (B) perform a compensating removal (call the
appropriate _recurringJobManager.RemoveIfExists or equivalent) if
SaveChangesAsync fails, then rethrow or return a success/failure DTO consistent
with the actual state; reference the ResumeJobAsync method, the
_recurringJobManager.AddOrUpdate call, and the _context.SaveChangesAsync call to
locate where to add the widened catch or compensation logic.
- Around line 193-222: Change ResumeJobAsync to accept a nullable row-version
and guard against null the same way PauseJobAsync does: update the parameter to
byte[]? expectedRowVersion (nullable) and before calling
marker.RowVersion.SequenceEqual(expectedRowVersion) add a check that treats null
as a concurrency mismatch (e.g. if (expectedRowVersion == null ||
!marker.RowVersion.SequenceEqual(expectedRowVersion)) throw new
SchedulerConcurrencyException(id);). Reference ResumeJobAsync and
marker.RowVersion.SequenceEqual(expectedRowVersion) and ensure callers are
adjusted to pass nullable byte[] where needed.
- Around line 267-309: ReconcileAsync mixes EF deletions
(_context.SchedulerJobStates.Remove(marker)) with Hangfire side-effects
(_recurringJobManager.RemoveIfExists), but SaveChangesAsync is only called after
the loop if SystemMarkersDeleted>0, so a failure in RemoveIfExists can prevent
persisting marker deletes; fix by either (A) first collect and persist all
system-marker removals with SaveChangesAsync before performing Hangfire
mutations, or (B) wrap calls to
_recurringJobManager.RemoveIfExists(marker.RecurringJobId) in a try/catch
modeled on the PauseJobAsync error handling (catch, log sanitized id and
continue) so a flaky Hangfire call won’t abort the loop; update ReconcileAsync
accordingly and keep behavior of outcome counters consistent.
In `@web/Classes/ForwardedHeadersExtensions.cs`:
- Line 67: The log call writes an external CIDR value raw; update the warning in
ForwardedHeadersExtensions (the logger.Warn call that currently logs "Skipping
invalid Cloudflare CIDR: {Cidr}" with variable cidr) to sanitize the cidr before
logging by passing it through the LogSanitizer (e.g., replace cidr with
LogSanitizer.Sanitize(cidr) or the project’s equivalent Sanitize method) so the
external input is not written raw to logs while preserving the existing
exception and message.
In `@web/Classes/Scheduler/HangfireJobLoggingFilter.cs`:
- Around line 44-53: The current code in HangfireJobLoggingFilter.cs constructs
and logs raw argument values (using context.BackgroundJob.Job.Args, Truncate,
LogSanitizer) which can leak secrets/PII; change it to log only metadata: build
an argument summary (e.g., argument count and a comma-separated list of argument
type names or simple type names) instead of the values, optionally sanitize type
names with LogSanitizer, and pass that summary into the _logger.LogInformation
call (replace uses of args/Truncate/LogSanitizer on values). Ensure you update
the string interpolation placeholders (JobArgs) to reflect the new metadata and
remove any logging of actual argument values.
---
Duplicate comments:
In `@web/appsettings.json`:
- Around line 39-43: The Hangfire section contains line-comment blocks that
break the strict JSON parser; remove the standalone // comments between
top-level keys and replace them with a single trailing inline comment on the
"Enabled" property (inside the "Hangfire" object) using JSONC-style inline
comment syntax so the config reader can parse it (update the "Hangfire" object
that contains "Enabled" accordingly).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7cccd462-5d95-4874-bc77-997d16f21ae2
📒 Files selected for processing (43)
.env.local.exampletest/HealthChecks/HangfireHealthCheckTests.cstest/RAPS/RapsRoleRefreshScheduledJobTests.cstest/Scheduler/HangfireJobLoggingFilterTests.cstest/Scheduler/JobsControllerTests.cstest/Scheduler/ScheduledJobDiscoveryTests.cstest/Scheduler/ScheduledJobRunnerTests.cstest/Scheduler/SchedulerJobsServiceTests.cstest/Viper.test.csprojweb/Areas/RAPS/Jobs/RapsRoleRefreshScheduledJob.csweb/Areas/RAPS/Services/RAPSAuditService.csweb/Areas/RAPS/Services/RoleViews.csweb/Areas/Scheduler/Controllers/.gitkeepweb/Areas/Scheduler/Controllers/JobsController.csweb/Areas/Scheduler/Models/DTOs/Responses/PauseResumeResultDto.csweb/Areas/Scheduler/Models/DTOs/Responses/ReconcilerOutcomeDto.csweb/Areas/Scheduler/Models/DTOs/Responses/SchedulerJobDto.csweb/Areas/Scheduler/Models/Entities/SchedulerJobState.csweb/Areas/Scheduler/Models/ScheduledJobContext.csweb/Areas/Scheduler/Models/TriggerSource.csweb/Areas/Scheduler/README.mdweb/Areas/Scheduler/Services/.gitkeepweb/Areas/Scheduler/Services/IScheduledJob.csweb/Areas/Scheduler/Services/ISchedulerJobsService.csweb/Areas/Scheduler/Services/ScheduledJobAttribute.csweb/Areas/Scheduler/Services/ScheduledJobDiscovery.csweb/Areas/Scheduler/Services/ScheduledJobRegistry.csweb/Areas/Scheduler/Services/ScheduledJobRunner.csweb/Areas/Scheduler/Services/SchedulerExceptions.csweb/Areas/Scheduler/Services/SchedulerJobReconciler.csweb/Areas/Scheduler/Services/SchedulerJobsService.csweb/Classes/ForwardedHeadersExtensions.csweb/Classes/HealthChecks/HangfireHealthCheck.csweb/Classes/SQLContext/SchedulerContext.csweb/Classes/SQLContext/VIPERContext.csweb/Classes/Scheduler/HangfireDashboardAuthorizationFilter.csweb/Classes/Scheduler/HangfireExtensions.csweb/Classes/Scheduler/HangfireJobLoggingFilter.csweb/Classes/Scheduler/SchedulerSchemaInitializer.csweb/Program.csweb/Views/Shared/Components/MainNav/MainNav.csweb/Viper.csprojweb/appsettings.json
3b13973 to
81e7857
Compare
| new string[] { "~/Policy", "Policies", "SVMSecure" }, | ||
| new string[] { "/research/default.cfm", "Research", "SVMSecure.Research" }, | ||
| new string[] { "/schedule/default.cfm", "Schedule", "SVMSecure.Schedule" }, | ||
| new[] { "~/scheduler/dashboard", "Scheduler", "SVMSecure.CATS.scheduledJobs", "Hangfire scheduler dashboard" }, |
There was a problem hiding this comment.
Can we add a link to the hangfire dashboard to the left nav of the computing section instead?
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.env.local.example:
- Around line 18-23: Update the .env.local.example comment to reflect the actual
default for Hangfire: change the wording that currently says "Off by Default" to
indicate Hangfire is enabled by default (e.g., "On/Enabled by default") and
clarify that the toggle is Hangfire__Enabled (or Hangfire:Enabled) so local
operators know the runtime default is true; edit the block that describes
Hangfire (the comment lines referencing Hangfire__Enabled/Hangfire:Enabled) to
state the default is enabled and how to disable it.
In `@VueApp/src/layouts/LeftNav.vue`:
- Around line 165-179: The current isInSpaRoute function uses a fragile regex on
r.path to detect catch-all routes; update it to detect catch-all routes by
checking route identity instead (e.g., inspect router.resolve(url).matched and
treat a route as catch-all if its name equals the known catch-all name or it has
a specific meta flag like r.meta?.catchAll), and fall back to the existing regex
only if neither name nor meta is present; modify isInSpaRoute, the matched
variable usage and each matched route check (r.path/r.name/r.meta) accordingly
so catch-all detection is robust and can be driven by route name or meta instead
of relying solely on /\(\.\*\)/.test(r.path).
- Around line 278-281: The nav prefix is being uppercased
(props.nav.toUpperCase()) which breaks hyphenated module names; instead derive
the module segment by taking the last hyphen-separated token from props.nav
(props.nav.split('-').pop()), normalize it to a route-friendly form by
capitalizing only the first character and lowercasing the rest, then build the
candidate as `/${module}/${r.menuItemURL}` when isRelativeUrl and
props.nav/props.navarea are present; update the code that sets candidate (the
block using isRelativeUrl, props.nav, r.menuItemURL) to use this transformation
so hyphenated values like "viper-effort" map to "/Effort/…" and single-word
values continue to work.
In `@web/Areas/Scheduler/README.md`:
- Around line 123-126: Update the README text to reflect the real job API:
replace references to PerformContext with ScheduledJobContext and note that
scheduled jobs receive a ScheduledJobContext parameter and should call
context.WriteLine(...) to emit per-job console output; ensure the doc mentions
the correct type name ScheduledJobContext and the expected method WriteLine so
readers won't copy/paste the wrong PerformContext-based signature.
In `@web/Areas/Scheduler/Services/ScheduledJobRegistry.cs`:
- Around line 39-42: The constructor for ScheduledJobRegistry currently assigns
the caller's IReadOnlyDictionary reference to JobsById allowing external
mutations; modify the ScheduledJobRegistry(IReadOnlyDictionary<string,
ScheduledJobMetadata> jobsById) constructor to create a defensive copy (e.g.,
new Dictionary<string, ScheduledJobMetadata>(jobsById)) and assign a read-only
wrapper (e.g., new ReadOnlyDictionary<string, ScheduledJobMetadata>(copiedDict)
or jobsByIdCopy.AsReadOnly()) to the JobsById property so the registry snapshot
is immutable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9c0e7c23-6210-45f2-8d71-f0211b6f9609
📒 Files selected for processing (31)
.env.local.exampleVueApp/src/layouts/LeftNav.vuetest/HealthChecks/HangfireHealthCheckTests.cstest/RAPS/RapsRoleRefreshScheduledJobTests.cstest/Scheduler/HangfireJobLoggingFilterTests.cstest/Scheduler/ScheduledJobDiscoveryTests.cstest/Scheduler/ScheduledJobRunnerTests.cstest/Viper.test.csprojweb/Areas/RAPS/Jobs/RapsRoleRefreshScheduledJob.csweb/Areas/RAPS/Services/RAPSAuditService.csweb/Areas/RAPS/Services/RoleViews.csweb/Areas/Scheduler/Controllers/.gitkeepweb/Areas/Scheduler/Models/ScheduledJobContext.csweb/Areas/Scheduler/Models/TriggerSource.csweb/Areas/Scheduler/README.mdweb/Areas/Scheduler/Services/.gitkeepweb/Areas/Scheduler/Services/IScheduledJob.csweb/Areas/Scheduler/Services/ScheduledJobAttribute.csweb/Areas/Scheduler/Services/ScheduledJobDiscovery.csweb/Areas/Scheduler/Services/ScheduledJobRegistry.csweb/Areas/Scheduler/Services/ScheduledJobRunner.csweb/Classes/ForwardedHeadersExtensions.csweb/Classes/HealthChecks/HangfireHealthCheck.csweb/Classes/Scheduler/HangfireDashboardAuthorizationFilter.csweb/Classes/Scheduler/HangfireExtensions.csweb/Classes/Scheduler/HangfireJobLoggingFilter.csweb/Program.csweb/Views/Shared/Components/MainNav/MainNav.csweb/Viper.csprojweb/appsettings.Development.jsonweb/appsettings.json
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
web/appsettings.json (1)
39-42:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStandalone
//comment between properties breaks Biome parser.Line 40 contains an invalid JSON property (standalone comment). ASP.NET Core's config reader permits it, but Biome and standard JSON tooling reject it. Move the comment inline on line 41 or remove it.
🛠️ Proposed fix
}, "Hangfire": { - // Hangfire's tables live in ConnectionStrings:VIPER. - "Enabled": true + "Enabled": true // Tables live in ConnectionStrings:VIPER }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/appsettings.json` around lines 39 - 42, The standalone // comment between properties in web/appsettings.json (inside the "Hangfire" object) breaks the Biome JSON parser; remove that standalone comment line or convert it to a JSON-safe form (e.g., delete it or add a descriptive property like "_comment": "Hangfire's tables live in ConnectionStrings:VIPER") and ensure the "Hangfire" object remains valid JSON with "Enabled": true; update the "Hangfire" block so no standalone // lines exist.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/Areas/Scheduler/README.md`:
- Around line 39-43: The example in RunAsync omits passing the CancellationToken
to UpdateRoles; update the call in RunAsync to pass ct (i.e., call
UpdateRoles(modBy: context.ModBy, debugOnly: false, ct: ct)) so the
cancellation-token pattern is preserved — locate the RunAsync method, the
RoleViews instantiation and the UpdateRoles invocation to add the ct argument
(RoleViews, UpdateRoles, RunAsync, ScheduledJobContext, ct).
---
Duplicate comments:
In `@web/appsettings.json`:
- Around line 39-42: The standalone // comment between properties in
web/appsettings.json (inside the "Hangfire" object) breaks the Biome JSON
parser; remove that standalone comment line or convert it to a JSON-safe form
(e.g., delete it or add a descriptive property like "_comment": "Hangfire's
tables live in ConnectionStrings:VIPER") and ensure the "Hangfire" object
remains valid JSON with "Enabled": true; update the "Hangfire" block so no
standalone // lines exist.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f9e77017-39c2-4e20-8753-90a78e418eb1
📒 Files selected for processing (11)
.env.local.exampleVueApp/src/layouts/LeftNav.vuetest/Scheduler/HangfireJobLoggingFilterTests.cstest/Scheduler/ScheduledJobRunnerTests.csweb/Areas/RAPS/Jobs/RapsRoleRefreshScheduledJob.csweb/Areas/RAPS/Services/RoleViews.csweb/Areas/Scheduler/Models/ScheduledJobContext.csweb/Areas/Scheduler/README.mdweb/Areas/Scheduler/Services/ScheduledJobRunner.csweb/Classes/Scheduler/HangfireJobLoggingFilter.csweb/appsettings.json
…left nav The shared LeftNav.vue treated every non-http URL as in-SPA and used Vue Router for navigation. URLs pointing outside the current SPA (e.g. /2/scheduler/dashboard from the Computing SPA) fell through to the catch-all 404 route while the URL bar silently updated, so clicks appeared to navigate but actually went nowhere. - Add isInSpaRoute() helper that resolves the URL against the SPA's registered routes and rejects matches that hit only a Vue Router 4 catch-all regex segment. - URLs that don't match a real SPA route now render as plain anchors via internalAnchorUrl, with router.resolve(...).href so the SPA's base path (e.g. /2/) is restored before the href is written (the LayoutController strips /2 before returning URLs to the SPA). - Gate target="_blank" on isExternalSite so same-host anchors stay in the same tab. The bug pre-dates this PR; it surfaces now because the scheduler is the first data-driven left-nav entry that points at a non-SPA route.
Adds a shared, area-agnostic background-job scheduler on top of Hangfire 1.8 + SQL Server. Jobs implement a thin IScheduledJob and are auto-discovered. A CAS+RAPS-gated Hangfire dashboard sits on top. The RAPS nightly role-refresh is the first consumer; the legacy ColdFusion runner remains as a debug-only manual fallback. - Hangfire.SqlServer storage lives in the VIPER DB under the [HangFire] schema. Hangfire:Enabled (default true) is the master switch; Hangfire:AutoSchedule (dev: false) registers jobs with Cron.Never so developers don't run cron locally. - Dashboard at /scheduler/dashboard is gated by SVMSecure.CATS.scheduledJobs — same permission the legacy ColdFusion scheduler uses, so existing CATS admins inherit access. Hangfire.Console and Hangfire.Heartbeat plugins surface per-job logs and CPU/RAM metrics in the dashboard. - Jobs implement IScheduledJob and carry [ScheduledJob(id, cron)]. A startup discovery pass scans the web assembly, registers each type with DI, and AddOrUpdate's the recurring job through a single ScheduledJobRunner dispatcher (Hangfire can't serialize calls against interfaces). Each execution gets a fresh DI scope and a ScheduledJobContext whose ModBy is "__sched" (7 chars to fit the legacy tblRoleMembers.ModBy varchar(8) column, distinct from the existing "__system" convention). - Hangfire health check tagged "ready" piggybacks on /health/detail: reports Healthy / Degraded / Unhealthy based on storage reachability and recent server heartbeats. - RAPS role-refresh (raps:role-refresh, "0 0 * * *" Pacific) wraps RoleViews.UpdateRoles. UpdateRoles now accepts an explicit modBy so scheduled and manual paths stamp audit rows distinguishably without UserHelper.GetCurrentUser(), and honors the CancellationToken at the per-role iteration boundary so app-pool recycles or deploys cancel cleanly mid-run. - Scheduler nav: /scheduler/* is mapped to the "Computing" top tab so visiting the dashboard highlights the right top-nav item. The actual link lives in the data-driven Computing left nav under the existing "System Diagnostic Tools" cluster (added via the CMS left-nav editor), alongside the legacy scheduler entries. - Program.cs Cloudflare/F5 forwarded-headers block is extracted to ForwardedHeadersExtensions to keep Main$ below CA1502/CA1505 limits with the new AddViperHangfire wiring added alongside it.
Hangfire's dashboard renders AppPath verbatim in the navbar link. We have no UsePathBase middleware, so the /2 deployment prefix used in TEST/PROD has to come from configuration. Default the AppPath to /Computing (correct for local dev where there's no prefix), and override to /2/Computing in appsettings.Test/Production so the link lands on the VIPER2 Computing area instead of the site root.
Hangfire's dashboard middleware writes a bare 403 directly to the response when our IDashboardAuthorizationFilter returns false, which bypasses cookie auth's AccessDeniedPath and lands the user on the browser's default error page. Add UseStatusCodePagesWithReExecute so any bare 4xx/5xx status code re-runs through HomeController.Error and the existing /Views/Home/403.cshtml view. Applies globally — also catches stray bare 404s from any other middleware. Status code is preserved on the response (browser still sees 403); only the body is replaced with the rendered view.
|
@bsedwards I added a "Scheduler V2" under "System Diagnostic Tools" at https://secure-test.vetmed.ucdavis.edu/2/Computing |
Summary
Adds a shared, area-agnostic background-job scheduler on top of Hangfire 1.8 + SQL Server. Jobs implement a thin
IScheduledJoband are auto-discovered. A CAS+RAPS-gated Hangfire dashboard sits on top. The RAPS nightly role-refresh is the first consumer.Hangfire:Enableddefaults totrue; flip tofalsein any environment to unwire everything.What an admin sees
/scheduler/dashboard. Gated bySVMSecure.CATS.scheduledJobs(same permission as the legacy ColdFusion scheduler, so existing admins inherit access). Shows recurring jobs, queues, history, per-job execution console, and per-server CPU/RAM via the bundled Hangfire.Console + Hangfire.Heartbeat plugins.hangfireon/health/detail: storage reachability + heartbeat freshness.What a job author sees
Discovery scans the web assembly at startup, registers each job with DI, and wires Hangfire via a single dispatcher. Every run gets a fresh DI scope and a
ScheduledJobContextcarryingTriggerSource(Scheduled/Manual) and the effectiveModBy. Jobs never callUserHelper.GetCurrentUser().Configuration
Hangfire:EnabledtrueHangfire:AutoScheduletrue(dev:false)Cron.Neverso cron doesn't fire. Worker still runs and "Trigger now" still works. Local dev opts out so developers don't run cron jobs.ConnectionStrings:VIPERWhere to focus review
HangfireExtensions.cs: the wiring spine (DI, server, dashboard, plugins). All flag handling lives here.ScheduledJobDiscovery.cs+ScheduledJobRunner.cs: auto-registration + Hangfire dispatch.web/Areas/Scheduler/README.md: onboarding recipe + ops runbook.Decisions worth flagging
__schedaudit stamp (not__scheduler). LegacytblRoleMembers.ModByisvarchar(8), so the canonical 11-char stamp truncated.Hangfire:Enabled=false. Matches the legacy "Computing" link behavior; documented in the README.TEST database cleanup
Earlier branch commits deployed a
[HangFire].[SchedulerJobState]marker table and registered a__scheduler:reconcilerecurring job to TEST as part of a pause/resume API that has since been removed (YAGNI — Hangfire's dashboard provides "Trigger now" / "Requeue" andHangfire:AutoSchedule=falsecovers env-wide off-switch needs). Before deploying this branch to TEST, run:Test plan
raps:role-refreshtriggered manually surfaces all 48 per-role messages in the dashboard consoleraps:role-refresh, confirm it runs successfullySummary by CodeRabbit
New Features
/scheduler/dashboardfor job monitoring and managementDocumentation
Tests
Chores