Skip to content

VPR-54 feat(scheduler): Hangfire-backed scheduler#182

Open
rlorenzo wants to merge 4 commits into
mainfrom
VPR-54-hangfire
Open

VPR-54 feat(scheduler): Hangfire-backed scheduler#182
rlorenzo wants to merge 4 commits into
mainfrom
VPR-54-hangfire

Conversation

@rlorenzo
Copy link
Copy Markdown
Contributor

@rlorenzo rlorenzo commented May 7, 2026

Summary

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.

Hangfire:Enabled defaults to true; flip to false in any environment to unwire everything.

What an admin sees

  • Dashboard at /scheduler/dashboard. Gated by SVMSecure.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.
  • Health check hangfire on /health/detail: storage reachability + heartbeat freshness.

What a job author sees

[ScheduledJob(id: "raps:role-refresh", cron: "0 0 * * *", TimeZoneId = "Pacific Standard Time")]
public sealed class RapsRoleRefreshScheduledJob(RAPSContext ctx, ILogger<...> log) : IScheduledJob
{
    public async Task RunAsync(ScheduledJobContext context, CancellationToken ct)
    {
        context.WriteLine("Starting...");          // surfaces in dashboard console
        await new RoleViews(ctx).UpdateRoles(modBy: context.ModBy);
    }
}

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 ScheduledJobContext carrying TriggerSource (Scheduled/Manual) and the effective ModBy. Jobs never call UserHelper.GetCurrentUser().

Configuration

Key Default Purpose
Hangfire:Enabled true Master switch. False = nothing wires up.
Hangfire:AutoSchedule true (dev: false) When false, jobs register with Cron.Never so cron doesn't fire. Worker still runs and "Trigger now" still works. Local dev opts out so developers don't run cron jobs.
ConnectionStrings:VIPER n/a Hangfire's tables live here.

Where 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

  • Hangfire.SqlServer in the existing VIPER DB, no in-memory phase. Hangfire's tables auto-migrate on first server start.
  • __sched audit stamp (not __scheduler). Legacy tblRoleMembers.ModBy is varchar(8), so the canonical 11-char stamp truncated.
  • Scheduler nav tab shown whenever the user has the permission, even if 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:reconcile recurring job to TEST as part of a pause/resume API that has since been removed (YAGNI — Hangfire's dashboard provides "Trigger now" / "Requeue" and Hangfire:AutoSchedule=false covers env-wide off-switch needs). Before deploying this branch to TEST, run:

USE viper_test;  -- or whichever DB hosts TEST's HangFire schema
GO

IF EXISTS (
    SELECT 1 FROM sys.tables t
    INNER JOIN sys.schemas s ON t.schema_id = s.schema_id
    WHERE s.name = 'HangFire' AND t.name = 'SchedulerJobState')
BEGIN
    DROP TABLE [HangFire].[SchedulerJobState];
END
GO

-- Remove the orphan reconciler recurring job from Hangfire storage so it
-- doesn't linger in the dashboard after this PR removes its registration code.
DELETE FROM [HangFire].[Hash] WHERE [Key] = 'recurring-job:__scheduler:reconcile';
DELETE FROM [HangFire].[Set]  WHERE [Key] = 'recurring-jobs' AND [Value] = '__scheduler:reconcile';
GO

Test plan

  • Local: dashboard renders, heartbeat populates (CPU/RAM after ~30s), raps:role-refresh triggered manually surfaces all 48 per-role messages in the dashboard console
  • Backend tests pass (1966)
  • ReSharper PR-scoped gate clean
  • TEST smoke: run the SQL cleanup above, deploy, manually trigger raps:role-refresh, confirm it runs successfully

Summary by CodeRabbit

  • New Features

    • Implemented background job scheduler with SQL Server storage integration
    • Added automated daily RAPS role refresh job
    • Added scheduler dashboard accessible at /scheduler/dashboard for job monitoring and management
    • Added health checks for scheduler operational monitoring
  • Documentation

    • Added operational guide covering scheduler configuration, dashboard access, and troubleshooting
  • Tests

    • Added comprehensive test coverage for scheduler functionality
  • Chores

    • Added scheduler-related NuGet dependencies
    • Updated configuration to support scheduler enablement and auto-scheduling options

Review Change Stack

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 7, 2026

Bundle Report

Changes will increase total bundle size by 215 bytes (0.01%) ⬆️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
viper-frontend-esm 2.14MB 215 bytes (0.01%) ⬆️

Affected Assets, Files, and Routes:

view changes for bundle: viper-frontend-esm

Assets Changed:

Asset Name Size Change Total Size Change (%)
assets/GenericError-*.js 215 bytes 21.32kB 1.02%

Files in assets/GenericError-*.js:

  • ./src/layouts/LeftNav.vue → Total Size: 122 bytes

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 7, 2026

Codecov Report

❌ Patch coverage is 45.47945% with 199 lines in your changes missing coverage. Please review.
✅ Project coverage is 43.01%. Comparing base (858333b) to head (b57055e).
⚠️ Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
web/Classes/Scheduler/HangfireExtensions.cs 0.00% 51 Missing ⚠️
.../Areas/Scheduler/Services/ScheduledJobDiscovery.cs 45.90% 31 Missing and 2 partials ⚠️
web/Classes/ForwardedHeadersExtensions.cs 0.00% 33 Missing ⚠️
web/Areas/RAPS/Jobs/RapsRoleRefreshScheduledJob.cs 0.00% 21 Missing ⚠️
.../Scheduler/HangfireDashboardAuthorizationFilter.cs 0.00% 21 Missing ⚠️
VueApp/src/layouts/LeftNav.vue 0.00% 13 Missing ⚠️
web/Areas/RAPS/Services/RoleViews.cs 0.00% 11 Missing ⚠️
web/Areas/Scheduler/Models/ScheduledJobContext.cs 50.00% 5 Missing and 1 partial ⚠️
web/Program.cs 0.00% 4 Missing ⚠️
...b/Areas/Scheduler/Services/ScheduledJobRegistry.cs 90.00% 2 Missing ⚠️
... and 3 more
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     
Flag Coverage Δ
backend 43.10% <47.15%> (+0.07%) ⬆️
frontend 41.19% <0.00%> (-0.16%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rlorenzo
Copy link
Copy Markdown
Contributor Author

rlorenzo commented May 7, 2026

@CodeRabbit review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 7, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 7, 2026

📝 Walkthrough

Walkthrough

This PR implements a complete Hangfire-based background job scheduler for VIPER. It adds job discovery via reflection and [ScheduledJob] attributes, per-execution context and audit stamping via ScheduledJobContext, structured logging with scope management, health monitoring, and integrates RAPS nightly role-refresh reconciliation with scheduler-driven audit actors. Configuration gates the scheduler via Hangfire:Enabled, and the dashboard is access-controlled by permission.

Changes

Hangfire Scheduler Feature

Layer / File(s) Summary
Scheduler Data Models and Context
web/Areas/Scheduler/Models/ScheduledJobContext.cs, web/Areas/Scheduler/Services/IScheduledJob.cs, web/Areas/Scheduler/Services/ScheduledJobAttribute.cs, web/Areas/Scheduler/Services/ScheduledJobRegistry.cs
ScheduledJobContext carries per-execution audit actor and optional Hangfire console output; ScheduledJobAttribute enables discovery with id/cron/timezone; IScheduledJob interface and registry classes define the job abstraction and metadata snapshot.
Job Discovery and DI Registration
web/Areas/Scheduler/Services/ScheduledJobDiscovery.cs
Reflection-based discovery of IScheduledJob implementations, validation of ScheduledJobAttribute requirement and unique ids, registration as Scoped services, creation of IScheduledJobRegistry singleton, and wiring of Hangfire recurring jobs with timezone resolution.
Job Execution with Logging and Scope Management
web/Areas/Scheduler/Services/ScheduledJobRunner.cs, web/Classes/Scheduler/HangfireJobLoggingFilter.cs
ScheduledJobRunner creates per-execution DI scope and invokes IScheduledJob with ScheduledJobContext stamped as SchedulerActor; HangfireJobLoggingFilter begins scope with jobId/recurringJobId, logs sanitized start/completion, and disposes scope during OnPerformed.
Hangfire Health Check Monitoring
web/Classes/HealthChecks/HangfireHealthCheck.cs
Queries Hangfire storage for server statistics and heartbeats; returns Healthy when recent heartbeats present (with metrics payload), Degraded when no servers registered, Unhealthy when heartbeats stale or storage unreachable.
Hangfire Service and Middleware Setup
web/Classes/Scheduler/HangfireExtensions.cs, web/Classes/Scheduler/HangfireDashboardAuthorizationFilter.cs
Conditionally registers Hangfire based on Hangfire:Enabled configuration; configures SQL Server storage with HangFire schema; enables console and heartbeat plugins; discovers and registers recurring jobs; mounts dashboard at /scheduler/dashboard with permission-based authorization filter.
RAPS Role Refresh Job and Audit Stamping
web/Areas/RAPS/Jobs/RapsRoleRefreshScheduledJob.cs, web/Areas/RAPS/Services/RoleViews.cs, web/Areas/RAPS/Services/RAPSAuditService.cs
RapsRoleRefreshScheduledJob implements IScheduledJob for nightly role reconciliation; RoleViews refactored to accept and thread optional modBy parameter through role member mutations; RAPSAuditService.AuditRoleMemberChange accepts optional modBy for non-user-context audit stamping with "__sched" actor.
Program.cs, Forwarded Headers, and Configuration
web/Program.cs, web/Classes/ForwardedHeadersExtensions.cs, web/appsettings.json, web/appsettings.Development.json
Program.cs adds Hangfire service and middleware registration; ForwardedHeadersExtensions centralizes proxy configuration (Cloudflare CIDRs, F5 IP, forward limits) extracted from inline logic; appsettings files add Hangfire:Enabled=true and per-environment AutoSchedule toggles.
MainNav and Vue Navigation Updates
web/Views/Shared/Components/MainNav/MainNav.cs, VueApp/src/layouts/LeftNav.vue
MainNav adds scheduler segment recognition for top-nav selection; LeftNav introduces isInSpaRoute helper to distinguish registered SPA routes, conditionally sets external-link attributes, maps menu items to router-link or plain anchor URLs.
Scheduler Operations Documentation
web/Areas/Scheduler/README.md, .env.local.example
README documents job onboarding, discovery/DI wiring, audit actor sourcing, configuration keys, access control, dashboard plugins, operations runbook, and code mapping; .env.local.example documents Hangfire__Enabled toggle.
NuGet Dependencies and Project Structure
test/Viper.test.csproj, web/Viper.csproj
Test project adds Hangfire.Core v1.8.23; main project adds Hangfire.AspNetCore, Hangfire.SqlServer (v1.8.23), Hangfire.Console, Hangfire.Heartbeat; explicit Scheduler folder entries in .csproj.
HangfireHealthCheck Test Coverage
test/HealthChecks/HangfireHealthCheckTests.cs
Tests verify Healthy outcome with recent heartbeats and metrics payload, Degraded when no servers registered, Unhealthy when heartbeats stale or storage unreachable, exception handling with "unreachable" description.
HangfireJobLoggingFilter Test Coverage
test/Scheduler/HangfireJobLoggingFilterTests.cs
Tests verify OnPerforming logs start message and begins scope with jobId/recurringJobId, OnPerformed logs completion or error with exact exception instance, scope disposal timing, and graceful handling when scope missing.
Job Discovery and Runner Test Coverage
test/Scheduler/ScheduledJobDiscoveryTests.cs, test/Scheduler/ScheduledJobRunnerTests.cs
Discovery tests verify reflection-based discovery, ScheduledJobAttribute requirement, duplicate-id detection, and validation errors; Runner tests verify job resolution by id, context stamping with SchedulerActor, and graceful skip of unknown ids.
RapsRoleRefreshScheduledJob Attribute Test
test/RAPS/RapsRoleRefreshScheduledJobTests.cs
Reflection-based test verifies RapsRoleRefreshScheduledJob has ScheduledJobAttribute with correct id, cron, and timezone.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • ucdavis/VIPER#159: Adds CloudflareNetworks static fetch method used in the new ForwardedHeadersExtensions for populating trusted proxy CIDRs.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 19.35% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed Title directly describes the main changeset: introducing a Hangfire-backed scheduler system. It is specific, clear, and reflects the primary architectural addition.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch VPR-54-hangfire

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Comment thread web/Areas/Scheduler/Services/SchedulerJobReconciler.cs Fixed
Comment thread web/Areas/Scheduler/Services/SchedulerJobsService.cs Fixed
Comment thread web/Classes/HealthChecks/HangfireHealthCheck.cs
Comment thread web/Classes/Scheduler/SchedulerSchemaInitializer.cs Fixed
Comment thread web/Classes/Scheduler/HangfireJobLoggingFilter.cs Fixed
Comment thread web/Areas/Scheduler/Services/ScheduledJobDiscovery.cs Fixed
Comment thread test/Scheduler/JobsControllerTests.cs Fixed
Comment thread test/Scheduler/JobsControllerTests.cs Fixed
Comment thread web/Areas/Scheduler/Services/IScheduledJob.cs Fixed
Comment thread web/Areas/Scheduler/Services/ScheduledJobDiscovery.cs Fixed
Comment thread web/Areas/Scheduler/Services/SchedulerJobReconciler.cs Fixed
Comment thread web/Areas/Scheduler/Services/SchedulerJobsService.cs Fixed
Comment thread web/Classes/HealthChecks/HangfireHealthCheck.cs
Comment thread web/Classes/Scheduler/HangfireJobLoggingFilter.cs Fixed
Comment thread web/Classes/Scheduler/SchedulerSchemaInitializer.cs Fixed
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

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

📥 Commits

Reviewing files that changed from the base of the PR and between 3765296 and 8dfd6f9.

📒 Files selected for processing (43)
  • .env.local.example
  • test/HealthChecks/HangfireHealthCheckTests.cs
  • test/RAPS/RapsRoleRefreshScheduledJobTests.cs
  • test/Scheduler/HangfireJobLoggingFilterTests.cs
  • test/Scheduler/JobsControllerTests.cs
  • test/Scheduler/ScheduledJobDiscoveryTests.cs
  • test/Scheduler/ScheduledJobRunnerTests.cs
  • test/Scheduler/SchedulerJobsServiceTests.cs
  • test/Viper.test.csproj
  • web/Areas/RAPS/Jobs/RapsRoleRefreshScheduledJob.cs
  • web/Areas/RAPS/Services/RAPSAuditService.cs
  • web/Areas/RAPS/Services/RoleViews.cs
  • web/Areas/Scheduler/Controllers/.gitkeep
  • web/Areas/Scheduler/Controllers/JobsController.cs
  • web/Areas/Scheduler/Models/DTOs/Responses/PauseResumeResultDto.cs
  • web/Areas/Scheduler/Models/DTOs/Responses/ReconcilerOutcomeDto.cs
  • web/Areas/Scheduler/Models/DTOs/Responses/SchedulerJobDto.cs
  • web/Areas/Scheduler/Models/Entities/SchedulerJobState.cs
  • web/Areas/Scheduler/Models/ScheduledJobContext.cs
  • web/Areas/Scheduler/Models/TriggerSource.cs
  • web/Areas/Scheduler/README.md
  • web/Areas/Scheduler/Services/.gitkeep
  • web/Areas/Scheduler/Services/IScheduledJob.cs
  • web/Areas/Scheduler/Services/ISchedulerJobsService.cs
  • web/Areas/Scheduler/Services/ScheduledJobAttribute.cs
  • web/Areas/Scheduler/Services/ScheduledJobDiscovery.cs
  • web/Areas/Scheduler/Services/ScheduledJobRegistry.cs
  • web/Areas/Scheduler/Services/ScheduledJobRunner.cs
  • web/Areas/Scheduler/Services/SchedulerExceptions.cs
  • web/Areas/Scheduler/Services/SchedulerJobReconciler.cs
  • web/Areas/Scheduler/Services/SchedulerJobsService.cs
  • web/Classes/ForwardedHeadersExtensions.cs
  • web/Classes/HealthChecks/HangfireHealthCheck.cs
  • web/Classes/SQLContext/SchedulerContext.cs
  • web/Classes/SQLContext/VIPERContext.cs
  • web/Classes/Scheduler/HangfireDashboardAuthorizationFilter.cs
  • web/Classes/Scheduler/HangfireExtensions.cs
  • web/Classes/Scheduler/HangfireJobLoggingFilter.cs
  • web/Classes/Scheduler/SchedulerSchemaInitializer.cs
  • web/Program.cs
  • web/Views/Shared/Components/MainNav/MainNav.cs
  • web/Viper.csproj
  • web/appsettings.json

Comment thread test/Scheduler/JobsControllerTests.cs Outdated
Comment thread test/Viper.test.csproj Outdated
Comment thread web/appsettings.json Outdated
Comment thread web/Areas/RAPS/Services/RAPSAuditService.cs Outdated
Comment thread web/Areas/RAPS/Services/RoleViews.cs Outdated
Comment thread web/Areas/Scheduler/Services/SchedulerJobsService.cs Outdated
Comment thread web/Classes/Scheduler/HangfireDashboardAuthorizationFilter.cs Outdated
Comment thread web/Classes/Scheduler/HangfireExtensions.cs
Comment thread web/Classes/Scheduler/SchedulerSchemaInitializer.cs Outdated
Comment thread web/Viper.csproj Outdated
Comment thread web/Areas/Scheduler/Services/SchedulerJobReconciler.cs Fixed
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

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.

Comment thread web/Classes/Scheduler/HangfireExtensions.cs
Comment thread web/Classes/Scheduler/HangfireExtensions.cs
Comment thread web/Classes/Scheduler/HangfireExtensions.cs Outdated
Comment thread web/Areas/Scheduler/Models/DTOs/Responses/SchedulerJobDto.cs Outdated
Comment thread web/Areas/Scheduler/Services/SchedulerJobReconciler.cs Fixed
Comment thread web/Areas/Scheduler/Services/SchedulerJobReconciler.cs Fixed
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 41 out of 43 changed files in this pull request and generated 2 comments.

Comment thread web/Areas/Scheduler/Services/SchedulerJobsService.cs Outdated
Comment thread web/Program.cs
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 41 out of 43 changed files in this pull request and generated 5 comments.

Comment thread web/Classes/Scheduler/HangfireExtensions.cs Outdated
Comment thread web/Areas/Scheduler/Services/SchedulerJobReconciler.cs Outdated
Comment thread web/Areas/Scheduler/Services/ScheduledJobDiscovery.cs Outdated
Comment thread web/Areas/RAPS/Services/RoleViews.cs Outdated
Comment thread web/Classes/Scheduler/HangfireDashboardAuthorizationFilter.cs
Comment thread web/Areas/Scheduler/Services/SchedulerJobReconciler.cs Fixed
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 41 out of 43 changed files in this pull request and generated 3 comments.

Comment thread web/Classes/Scheduler/HangfireExtensions.cs Outdated
Comment thread web/Classes/Scheduler/HangfireExtensions.cs Outdated
Comment thread web/Classes/ForwardedHeadersExtensions.cs
@rlorenzo rlorenzo force-pushed the VPR-54-hangfire branch from 4bd23f8 to 2843c64 Compare May 7, 2026 23:17
@rlorenzo rlorenzo requested a review from Copilot May 7, 2026 23:18
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 8

♻️ Duplicate comments (1)
web/appsettings.json (1)

39-43: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Biome 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

📥 Commits

Reviewing files that changed from the base of the PR and between 8dfd6f9 and a8ee33c.

📒 Files selected for processing (43)
  • .env.local.example
  • test/HealthChecks/HangfireHealthCheckTests.cs
  • test/RAPS/RapsRoleRefreshScheduledJobTests.cs
  • test/Scheduler/HangfireJobLoggingFilterTests.cs
  • test/Scheduler/JobsControllerTests.cs
  • test/Scheduler/ScheduledJobDiscoveryTests.cs
  • test/Scheduler/ScheduledJobRunnerTests.cs
  • test/Scheduler/SchedulerJobsServiceTests.cs
  • test/Viper.test.csproj
  • web/Areas/RAPS/Jobs/RapsRoleRefreshScheduledJob.cs
  • web/Areas/RAPS/Services/RAPSAuditService.cs
  • web/Areas/RAPS/Services/RoleViews.cs
  • web/Areas/Scheduler/Controllers/.gitkeep
  • web/Areas/Scheduler/Controllers/JobsController.cs
  • web/Areas/Scheduler/Models/DTOs/Responses/PauseResumeResultDto.cs
  • web/Areas/Scheduler/Models/DTOs/Responses/ReconcilerOutcomeDto.cs
  • web/Areas/Scheduler/Models/DTOs/Responses/SchedulerJobDto.cs
  • web/Areas/Scheduler/Models/Entities/SchedulerJobState.cs
  • web/Areas/Scheduler/Models/ScheduledJobContext.cs
  • web/Areas/Scheduler/Models/TriggerSource.cs
  • web/Areas/Scheduler/README.md
  • web/Areas/Scheduler/Services/.gitkeep
  • web/Areas/Scheduler/Services/IScheduledJob.cs
  • web/Areas/Scheduler/Services/ISchedulerJobsService.cs
  • web/Areas/Scheduler/Services/ScheduledJobAttribute.cs
  • web/Areas/Scheduler/Services/ScheduledJobDiscovery.cs
  • web/Areas/Scheduler/Services/ScheduledJobRegistry.cs
  • web/Areas/Scheduler/Services/ScheduledJobRunner.cs
  • web/Areas/Scheduler/Services/SchedulerExceptions.cs
  • web/Areas/Scheduler/Services/SchedulerJobReconciler.cs
  • web/Areas/Scheduler/Services/SchedulerJobsService.cs
  • web/Classes/ForwardedHeadersExtensions.cs
  • web/Classes/HealthChecks/HangfireHealthCheck.cs
  • web/Classes/SQLContext/SchedulerContext.cs
  • web/Classes/SQLContext/VIPERContext.cs
  • web/Classes/Scheduler/HangfireDashboardAuthorizationFilter.cs
  • web/Classes/Scheduler/HangfireExtensions.cs
  • web/Classes/Scheduler/HangfireJobLoggingFilter.cs
  • web/Classes/Scheduler/SchedulerSchemaInitializer.cs
  • web/Program.cs
  • web/Views/Shared/Components/MainNav/MainNav.cs
  • web/Viper.csproj
  • web/appsettings.json

Comment thread web/Areas/RAPS/Jobs/RapsRoleRefreshScheduledJob.cs
Comment thread web/Areas/Scheduler/README.md Outdated
Comment thread web/Areas/Scheduler/Services/ScheduledJobRegistry.cs
Comment thread web/Areas/Scheduler/Services/SchedulerJobsService.cs Outdated
Comment thread web/Areas/Scheduler/Services/SchedulerJobsService.cs Outdated
Comment thread web/Areas/Scheduler/Services/SchedulerJobsService.cs Outdated
Comment thread web/Classes/ForwardedHeadersExtensions.cs Outdated
Comment thread web/Classes/Scheduler/HangfireJobLoggingFilter.cs Outdated
@rlorenzo rlorenzo force-pushed the VPR-54-hangfire branch from a8ee33c to 97297cc Compare May 8, 2026 04:08
Comment thread web/Classes/Scheduler/HangfireJobLoggingFilter.cs Fixed
@rlorenzo rlorenzo force-pushed the VPR-54-hangfire branch 2 times, most recently from 3b13973 to 81e7857 Compare May 8, 2026 22:33
@rlorenzo rlorenzo requested a review from bsedwards May 9, 2026 05:15
@rlorenzo rlorenzo changed the title VPR-54 feat(scheduler): Hangfire-backed scheduler with pause/resume API VPR-54 feat(scheduler): Hangfire-backed scheduler May 11, 2026
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" },
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can we add a link to the hangfire dashboard to the left nav of the computing section instead?

@rlorenzo
Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 12, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

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

📥 Commits

Reviewing files that changed from the base of the PR and between a8ee33c and a55627e.

📒 Files selected for processing (31)
  • .env.local.example
  • VueApp/src/layouts/LeftNav.vue
  • test/HealthChecks/HangfireHealthCheckTests.cs
  • test/RAPS/RapsRoleRefreshScheduledJobTests.cs
  • test/Scheduler/HangfireJobLoggingFilterTests.cs
  • test/Scheduler/ScheduledJobDiscoveryTests.cs
  • test/Scheduler/ScheduledJobRunnerTests.cs
  • test/Viper.test.csproj
  • web/Areas/RAPS/Jobs/RapsRoleRefreshScheduledJob.cs
  • web/Areas/RAPS/Services/RAPSAuditService.cs
  • web/Areas/RAPS/Services/RoleViews.cs
  • web/Areas/Scheduler/Controllers/.gitkeep
  • web/Areas/Scheduler/Models/ScheduledJobContext.cs
  • web/Areas/Scheduler/Models/TriggerSource.cs
  • web/Areas/Scheduler/README.md
  • web/Areas/Scheduler/Services/.gitkeep
  • web/Areas/Scheduler/Services/IScheduledJob.cs
  • web/Areas/Scheduler/Services/ScheduledJobAttribute.cs
  • web/Areas/Scheduler/Services/ScheduledJobDiscovery.cs
  • web/Areas/Scheduler/Services/ScheduledJobRegistry.cs
  • web/Areas/Scheduler/Services/ScheduledJobRunner.cs
  • web/Classes/ForwardedHeadersExtensions.cs
  • web/Classes/HealthChecks/HangfireHealthCheck.cs
  • web/Classes/Scheduler/HangfireDashboardAuthorizationFilter.cs
  • web/Classes/Scheduler/HangfireExtensions.cs
  • web/Classes/Scheduler/HangfireJobLoggingFilter.cs
  • web/Program.cs
  • web/Views/Shared/Components/MainNav/MainNav.cs
  • web/Viper.csproj
  • web/appsettings.Development.json
  • web/appsettings.json

Comment thread .env.local.example Outdated
Comment thread VueApp/src/layouts/LeftNav.vue
Comment thread VueApp/src/layouts/LeftNav.vue
Comment thread web/Areas/Scheduler/README.md Outdated
Comment thread web/Areas/Scheduler/Services/ScheduledJobRegistry.cs
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 29 out of 32 changed files in this pull request and generated 2 comments.

Comment thread VueApp/src/layouts/LeftNav.vue
Comment thread web/appsettings.json
@rlorenzo rlorenzo requested a review from Copilot May 12, 2026 14:27
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 29 out of 32 changed files in this pull request and generated 3 comments.

Comment thread web/Areas/Scheduler/Services/ScheduledJobRunner.cs
Comment thread web/Classes/Scheduler/HangfireJobLoggingFilter.cs
Comment thread web/Areas/RAPS/Jobs/RapsRoleRefreshScheduledJob.cs
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 28 out of 31 changed files in this pull request and generated no new comments.

@rlorenzo
Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 12, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
web/appsettings.json (1)

39-42: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Standalone // 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

📥 Commits

Reviewing files that changed from the base of the PR and between a55627e and 414719b.

📒 Files selected for processing (11)
  • .env.local.example
  • VueApp/src/layouts/LeftNav.vue
  • test/Scheduler/HangfireJobLoggingFilterTests.cs
  • test/Scheduler/ScheduledJobRunnerTests.cs
  • web/Areas/RAPS/Jobs/RapsRoleRefreshScheduledJob.cs
  • web/Areas/RAPS/Services/RoleViews.cs
  • web/Areas/Scheduler/Models/ScheduledJobContext.cs
  • web/Areas/Scheduler/README.md
  • web/Areas/Scheduler/Services/ScheduledJobRunner.cs
  • web/Classes/Scheduler/HangfireJobLoggingFilter.cs
  • web/appsettings.json

Comment thread web/Areas/Scheduler/README.md
rlorenzo added 2 commits May 12, 2026 09:33
…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.
rlorenzo added 2 commits May 12, 2026 18:10
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.
@rlorenzo
Copy link
Copy Markdown
Contributor Author

@bsedwards I added a "Scheduler V2" under "System Diagnostic Tools" at https://secure-test.vetmed.ucdavis.edu/2/Computing

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.

5 participants