feat(jobs): add nextRun to JobSchedule and NEXT_RUN sort order#437
Merged
Conversation
- Add nextRun: Time! field to JobSchedule GraphQL type - Compute next run time from cron expression using robfig/cron/v3 - Add NEXT_RUN to JobOrderField enum for sorting jobs by next scheduled run - Jobs without a schedule sort last when ordering by NEXT_RUN - Add unit tests for Schedule() and nextRunUnix() - Include go fix changes
thokra-nav
approved these changes
May 13, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a computed nextRun timestamp to job schedules and introduces a NEXT_RUN ordering option so jobs can be sorted by their next scheduled execution time.
Changes:
- Added
JobSchedule.nextRunto the GraphQL schema and updated gqlgen-generated code accordingly. - Computed next run time from cron expression + timezone in the job model, and added
NEXT_RUNsorting support. - Applied Go stdlib modernizations (
maps.Copy,slices.Contains) and added unit tests for schedule/next-run behavior.
Reviewed changes
Copilot reviewed 8 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/workload/job/sortfilter.go | Registers NEXT_RUN sort order using a derived next-run timestamp. |
| internal/workload/job/schedule_test.go | Adds unit tests for Job.Schedule() and nextRunUnix(). |
| internal/workload/job/models.go | Computes JobSchedule.NextRun from cron + timezone (robfig/cron). |
| internal/workload/config/queries.go | Refactors annotation merging to use maps.Copy. |
| internal/kubernetes/fake/fake.go | Uses maps.Copy to merge metadata maps and switches to any. |
| internal/graph/schema/jobs.graphqls | Adds nextRun to JobSchedule and NEXT_RUN to JobOrderField. |
| internal/graph/gengql/root_.generated.go | Regenerated schema/complexity wiring for nextRun and NEXT_RUN. |
| internal/graph/gengql/jobs.generated.go | Regenerated job resolvers/marshaling for JobSchedule.nextRun. |
| internal/auth/middleware/kubernetes_serviceaccount.go | Simplifies JWT-shape check using slices.Contains. |
| go.mod | Adds robfig/cron dependency entry. |
| go.sum | Adds robfig/cron checksums. |
Files not reviewed (2)
- internal/graph/gengql/jobs.generated.go: Language not supported
- internal/graph/gengql/root_.generated.go: Language not supported
Comments suppressed due to low confidence (1)
internal/workload/job/models.go:356
nextRunmay end up as the zerotime.Timewhen cron parsing fails (err != nil). Since the GraphQLTimescalar marshaler returnsnullfor zero values, this will surface as a field error ifnextRunremains non-nullable in the schema. Consider either (a) returningnilschedule on parse errors, or (b) makingNextRuna*time.Time/ otherwise distinguishing "unknown" next run explicitly so GraphQL can represent it without error.
parser := cron.NewParser(cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow)
sched, err := parser.Parse(j.Spec.Schedule)
var nextRun time.Time
if err == nil {
nextRun = sched.Next(time.Now().In(loc))
}
return &JobSchedule{
Expression: j.Spec.Schedule,
TimeZone: tz,
NextRun: nextRun,
}
- Return nil schedule for invalid cron expressions (avoids null Time! field error) - Set TimeZone to UTC when LoadLocation fails (consistent with computed nextRun) - Switch NEXT_RUN to RegisterConcurrentSort (precomputes sort key once per job) - Mark robfig/cron/v3 as direct dependency via go mod tidy
rbjornstad
commented
May 13, 2026
Contributor
Author
rbjornstad
left a comment
rbjornstad
left a comment
There was a problem hiding this comment.
All review comments addressed in 8401a33.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 11 changed files in this pull request and generated 4 comments.
Files not reviewed (2)
- internal/graph/gengql/jobs.generated.go: Language not supported
- internal/graph/gengql/root_.generated.go: Language not supported
Comments suppressed due to low confidence (1)
internal/workload/job/schedule_test.go:60
- Same issue here:
new("Invalid/Zone")won’t compile;TimeZoneexpects a*string, so create a proper string pointer (e.g.ptr.To(...)or&s).
j := &Job{Spec: &nais_io_v1.NaisjobSpec{
Schedule: "0 * * * *",
TimeZone: new("Invalid/Zone"),
}}
Schedule() now always returns a JobSchedule when an expression exists, with nextRun: null when the cron expression fails to parse. This allows clients to always see the expression and timezone even for invalid schedules.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 11 changed files in this pull request and generated 3 comments.
Files not reviewed (2)
- internal/graph/gengql/jobs.generated.go: Language not supported
- internal/graph/gengql/root_.generated.go: Language not supported
Comments suppressed due to low confidence (1)
internal/workload/job/schedule_test.go:60
TimeZone: new("Invalid/Zone")does not compile in Go (the built-innewrequires a type, not a value). Use a string pointer helper to construct*stringvalues forNaisjobSpec.TimeZone.
j := &Job{Spec: &nais_io_v1.NaisjobSpec{
Schedule: "0 * * * *",
TimeZone: new("Invalid/Zone"),
}}
- Memoize Schedule() with sync.Once to avoid repeated cron parsing and ensure consistent nextRun across sort and field resolution - Switch NEXT_RUN to RegisterSort with explicit nil-last logic that keeps unscheduled jobs last regardless of sort direction (ASC/DESC) - Update description to say 'always sorted last'
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 11 changed files in this pull request and generated 2 comments.
Files not reviewed (2)
- internal/graph/gengql/jobs.generated.go: Language not supported
- internal/graph/gengql/root_.generated.go: Language not supported
The sortfilter framework applies DESC by swapping comparison arguments, which naturally inverts null ordering (standard SQL behavior). Updated the schema description to not promise direction-independent null ordering.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 12 changed files in this pull request and generated 2 comments.
Files not reviewed (2)
- internal/graph/gengql/jobs.generated.go: Language not supported
- internal/graph/gengql/root_.generated.go: Language not supported
Add partitionUnscheduledLast() that runs as a stable post-sort step after NEXT_RUN ordering, ensuring jobs without a valid schedule always appear at the end even when sorting DESC.
sync.Once makes Job contain sync.noCopy, which breaks gqlgen's generated type switches that use value type job.Job. Cron parsing is cheap and jobs are rebuilt from the K8s watcher per request, so memoization is unnecessary.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 13 changed files in this pull request and generated 3 comments.
Files not reviewed (2)
- internal/graph/gengql/jobs.generated.go: Language not supported
- internal/graph/gengql/root_.generated.go: Language not supported
Comments suppressed due to low confidence (1)
internal/workload/job/sortfilter_test.go:44
- Same flakiness risk as the ASC case: at certain times
* * * * *and0 * * * *can have equalnextRun, so the expected strict ordering may not hold. Use a fixed reference time or schedules that guarantee different next occurrences regardless of when the test runs.
t.Run("DESC sorts by next run time descending", func(t *testing.T) {
jobs := []*Job{
makeJob("every-minute", "* * * * *"),
makeJob("hourly", "0 * * * *"),
}
SortFilter.Sort(context.Background(), jobs, "NEXT_RUN", model.OrderDirectionDesc)
if jobs[0].Name != "hourly" || jobs[1].Name != "every-minute" {
t.Fatalf("expected [hourly, every-minute], got [%s, %s]", jobs[0].Name, jobs[1].Name)
}
- Store Schedule() result in local var in partitionUnscheduledLast - Use */5 instead of * * * * * to avoid hour-boundary coincidence
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 13 changed files in this pull request and generated 5 comments.
Files not reviewed (2)
- internal/graph/gengql/jobs.generated.go: Language not supported
- internal/graph/gengql/root_.generated.go: Language not supported
- Replace O(n log n) SortStableFunc partition with O(n) two-slice split - Schema description now mentions invalid cron as also sorting last - Test asserts both invalid-cron and no-schedule end up after valid jobs - PR description mismatch (sync.Once mention) noted for update
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 13 changed files in this pull request and generated 2 comments.
Files not reviewed (2)
- internal/graph/gengql/jobs.generated.go: Language not supported
- internal/graph/gengql/root_.generated.go: Language not supported
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add
nextRun: Time(nullable) toJobScheduleand make jobs sortable by next scheduled run time via theNEXT_RUNorder field.Changes
nextRun: TimetoJobScheduletype, addedNEXT_RUNtoJobOrderFieldenumrobfig/cron/v3, compute next run fromtime.Now()nextRunis nil when cron expression is invalid, so the expression and timezone are still visible