diff --git a/models/actions/runner.go b/models/actions/runner.go index bc4367b52e..7e8c2b644b 100644 --- a/models/actions/runner.go +++ b/models/actions/runner.go @@ -270,15 +270,31 @@ func (opts FindRunnerOptions) ToConds() builder.Cond { return cond } +// runnerStatusOrderExpr builds an ORDER BY fragment that ranks runners by their +// computed status (see ActionRunner.Status): active (0), idle (1), offline (2). +// The thresholds are evaluated against the current time, mirroring ToConds, so +// sorting by status groups active and idle runners instead of interleaving them +// by raw last_online. +func runnerStatusOrderExpr() string { + now := time.Now() + offlineThreshold := now.Add(-RunnerOfflineTime).Unix() + idleThreshold := now.Add(-RunnerIdleTime).Unix() + return fmt.Sprintf("CASE WHEN last_online <= %d THEN 2 WHEN last_active <= %d THEN 1 ELSE 0 END", offlineThreshold, idleThreshold) +} + func (opts FindRunnerOptions) ToOrders() string { // A unique tiebreaker (id) is appended so that runners sharing the same - // last_online or name keep a deterministic order across paginated queries, - // otherwise the same runner may appear on more than one page. + // status, last_online or name keep a deterministic order across paginated + // queries, otherwise the same runner may appear on more than one page. + statusRank := runnerStatusOrderExpr() switch opts.Sort { case "online": - return "last_online DESC, id ASC" + // Rank by computed status first so idle runners are not interleaved with + // active ones; disabled runners sink to the bottom of their status group + // (is_disabled ASC), then last_online breaks ties within a group. + return statusRank + " ASC, is_disabled ASC, last_online DESC, id ASC" case "offline": - return "last_online ASC, id ASC" + return statusRank + " DESC, is_disabled ASC, last_online ASC, id ASC" case "alphabetically": return "name ASC, id ASC" case "reversealphabetically": @@ -288,7 +304,7 @@ func (opts FindRunnerOptions) ToOrders() string { case "oldest": return "id ASC" } - return "last_online DESC, id ASC" + return statusRank + " ASC, is_disabled ASC, last_online DESC, id ASC" } // GetRunnerByUUID returns a runner via uuid diff --git a/models/actions/runner_test.go b/models/actions/runner_test.go index 47b8a69307..f7b4cb507b 100644 --- a/models/actions/runner_test.go +++ b/models/actions/runner_test.go @@ -4,16 +4,12 @@ package actions import ( - "fmt" "testing" "time" - "gitea.dev/models/db" - "gitea.dev/models/unittest" "gitea.dev/modules/timeutil" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) func TestShouldPersistLastOnline(t *testing.T) { @@ -85,65 +81,3 @@ func TestShouldPersistLastActive(t *testing.T) { }) } } - -func TestFindRunnerOptions_ToOrders_StableTiebreaker(t *testing.T) { - // Sorts on a non-unique column must end with the unique id tiebreaker so - // pagination is deterministic; without it, runners sharing the same - // last_online or name can appear on more than one page. Sorts already on - // the unique id need no tiebreaker. - expected := map[string]string{ - "": "last_online DESC, id ASC", - "online": "last_online DESC, id ASC", - "offline": "last_online ASC, id ASC", - "alphabetically": "name ASC, id ASC", - "reversealphabetically": "name DESC, id ASC", - "newest": "id DESC", - "oldest": "id ASC", - } - for sort, want := range expected { - assert.Equal(t, want, FindRunnerOptions{Sort: sort}.ToOrders(), "sort %q", sort) - } -} - -func TestFindRunners_PaginationNoDuplicates(t *testing.T) { - require.NoError(t, unittest.PrepareTestDatabase()) - ctx := t.Context() - - // Create several runners that all share the same last_online value so the - // primary sort key (last_online) is tied for all of them. - const ownerID = 1000 - const count = 6 - for i := range count { - runner := &ActionRunner{ - Name: "paginated-runner", - UUID: fmt.Sprintf("PAGINATE-TEST-0000-0000-00000000000%d", i), - TokenHash: fmt.Sprintf("paginate-test-token-hash-%d", i), - OwnerID: ownerID, - RepoID: 0, - LastOnline: 42, - } - require.NoError(t, db.Insert(ctx, runner)) - } - - // Page through the runners and ensure every id is returned exactly once. - seen := make(map[int64]int) - const pageSize = 2 - for page := 1; ; page++ { - runners, err := db.Find[ActionRunner](ctx, FindRunnerOptions{ - ListOptions: db.ListOptions{Page: page, PageSize: pageSize}, - OwnerID: ownerID, - }) - require.NoError(t, err) - if len(runners) == 0 { - break - } - for _, r := range runners { - seen[r.ID]++ - } - } - - assert.Len(t, seen, count, "each runner should be returned exactly once across all pages") - for id, n := range seen { - assert.Equal(t, 1, n, "runner %d appeared on %d pages", id, n) - } -} diff --git a/models/actions/task.go b/models/actions/task.go index 047db27761..cfb0c5ec06 100644 --- a/models/actions/task.go +++ b/models/actions/task.go @@ -85,11 +85,15 @@ func (task *ActionTask) IsStopped() bool { return task.Stopped > 0 } -func (task *ActionTask) GetRunLink() string { - if task.Job == nil || task.Job.Run == nil { +func (task *ActionTask) GetRunJobLink() string { + // Run.Repo can be nil when the repository was deleted while task/run rows remain + // (TaskList.LoadAttributes copies job.Repo into run.Repo, leaving it nil on a miss). + // Run.Link() already returns "" in that case, so guard here to avoid emitting a + // broken relative "/jobs/N" link from the Sprintf below. + if task.Job == nil || task.Job.Run == nil || task.Job.Run.Repo == nil { return "" } - return task.Job.Run.Link() + return fmt.Sprintf("%s/jobs/%d", task.Job.Run.Link(), task.Job.ID) } func (task *ActionTask) GetCommitLink() string { diff --git a/models/actions/task_test.go b/models/actions/task_test.go index 05f564f2b4..aaec01156c 100644 --- a/models/actions/task_test.go +++ b/models/actions/task_test.go @@ -9,6 +9,7 @@ import ( runnerv1 "gitea.dev/actions-proto-go/runner/v1" "gitea.dev/models/db" + repo_model "gitea.dev/models/repo" "gitea.dev/models/unittest" "gitea.dev/modules/actions/jobparser" "gitea.dev/modules/timeutil" @@ -18,6 +19,21 @@ import ( "google.golang.org/protobuf/types/known/timestamppb" ) +func TestActionTask_GetRunJobLink(t *testing.T) { + repo := &repo_model.Repository{OwnerName: "org", Name: "consumer"} + run := &ActionRun{ID: 10, Repo: repo} + job := &ActionRunJob{ID: 42, Run: run} + + // a task with a loaded job links to that specific job, not just the run + task := &ActionTask{Job: job} + assert.Equal(t, run.Link()+"/jobs/42", task.GetRunJobLink()) + + // missing job, run or repo yields an empty link instead of a broken URL + assert.Empty(t, (&ActionTask{}).GetRunJobLink()) + assert.Empty(t, (&ActionTask{Job: &ActionRunJob{ID: 42}}).GetRunJobLink()) + assert.Empty(t, (&ActionTask{Job: &ActionRunJob{ID: 42, Run: &ActionRun{ID: 10}}}).GetRunJobLink()) +} + func TestMakeTaskStepDisplayName(t *testing.T) { tests := []struct { name string diff --git a/options/locale/locale_en-US.json b/options/locale/locale_en-US.json index b98b05df24..599690891b 100644 --- a/options/locale/locale_en-US.json +++ b/options/locale/locale_en-US.json @@ -3741,7 +3741,7 @@ "actions.runners.runner_title": "Runner", "actions.runners.task_list": "Recent tasks on this runner", "actions.runners.task_list.no_tasks": "There is no task yet.", - "actions.runners.task_list.run": "Run", + "actions.runners.task_list.job": "Job", "actions.runners.task_list.status": "Status", "actions.runners.task_list.repository": "Repository", "actions.runners.task_list.commit": "Commit", diff --git a/templates/shared/actions/runner_edit.tmpl b/templates/shared/actions/runner_edit.tmpl index 76992eea52..fe55d84c65 100644 --- a/templates/shared/actions/runner_edit.tmpl +++ b/templates/shared/actions/runner_edit.tmpl @@ -65,7 +65,7 @@ - + @@ -75,7 +75,7 @@ {{range .Tasks}} - + {{end}} + - @@ -87,12 +87,12 @@ {{if $.AllowBulkActions}} {{end}} + -
{{ctx.Locale.Tr "actions.runners.task_list.run"}}{{ctx.Locale.Tr "actions.runners.task_list.job"}} {{ctx.Locale.Tr "actions.runners.task_list.status"}} {{ctx.Locale.Tr "actions.runners.task_list.repository"}} {{ctx.Locale.Tr "actions.runners.task_list.commit"}}
{{.ID}}{{if .Job}}{{.Job.ID}}{{else}}{{.ID}}{{end}} {{.Status.LocaleString ctx.Locale}} {{.GetRepoName}} diff --git a/templates/shared/actions/runner_list.tmpl b/templates/shared/actions/runner_list.tmpl index 03400fa3a7..4c615d25a5 100644 --- a/templates/shared/actions/runner_list.tmpl +++ b/templates/shared/actions/runner_list.tmpl @@ -62,6 +62,10 @@ {{if .AllowBulkActions}}
+ {{ctx.Locale.Tr "actions.runners.name"}} + {{SortArrow "alphabetically" "reversealphabetically" .SortType false}} + {{ctx.Locale.Tr "actions.runners.status"}} {{SortArrow "online" "offline" .SortType false}} @@ -70,10 +74,6 @@ {{ctx.Locale.Tr "actions.runners.id"}} {{SortArrow "oldest" "newest" .SortType false}} - {{ctx.Locale.Tr "actions.runners.name"}} - {{SortArrow "alphabetically" "reversealphabetically" .SortType false}} - {{ctx.Locale.Tr "actions.runners.version"}} {{ctx.Locale.Tr "actions.runners.owner_type"}} {{ctx.Locale.Tr "actions.runners.labels"}}

{{.Name}}

 - {{.StatusLocaleName ctx.Locale}} - {{if .IsDisabled}}{{ctx.Locale.Tr "actions.runners.disabled"}}{{end}} + {{.StatusLocaleName ctx.Locale}} + {{if .IsDisabled}}{{ctx.Locale.Tr "disabled"}}{{end}} {{.ID}}

{{.Name}}

 {{if .Version}}{{.Version}}{{else}}{{ctx.Locale.Tr "unknown"}}{{end}} {{.BelongsToOwnerType.LocaleString ctx.Locale}} diff --git a/tests/integration/actions_runner_sort_test.go b/tests/integration/actions_runner_sort_test.go new file mode 100644 index 0000000000..9c992146ef --- /dev/null +++ b/tests/integration/actions_runner_sort_test.go @@ -0,0 +1,130 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package integration + +import ( + "fmt" + "testing" + "time" + + actions_model "gitea.dev/models/actions" + "gitea.dev/models/db" + "gitea.dev/modules/timeutil" + "gitea.dev/tests" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestFindRunnersSortByStatus verifies that sorting by status ranks runners by +// their computed status (active, then idle, then offline) instead of interleaving +// idle runners with active ones by raw last_online, and that a disabled runner +// sinks to the bottom of its status group rather than mixing with enabled runners. +// +// It lives in tests/integration rather than a unit test because the status rank is +// a database-evaluated CASE expression; unit tests only run against SQLite, so the +// expression must be exercised against MySQL/PostgreSQL/MSSQL in CI to catch dialect +// differences. +func TestFindRunnersSortByStatus(t *testing.T) { + defer tests.PrepareTestEnv(t)() + ctx := t.Context() + + const ownerID = 1001 + now := time.Now() + // An idle runner that went online most recently would sort before an active + // runner when ordering by last_online alone; the status rank must override that. + insert := func(name string, lastOnline, lastActive time.Time, disabled bool) { + require.NoError(t, db.Insert(ctx, &actions_model.ActionRunner{ + Name: name, + UUID: "STATUS-SORT-" + name, + TokenHash: "status-sort-token-" + name, + OwnerID: ownerID, + LastOnline: timeutil.TimeStamp(lastOnline.Unix()), + LastActive: timeutil.TimeStamp(lastActive.Unix()), + IsDisabled: disabled, + })) + } + // Each disabled runner has the most recent last_online within its status group, + // so it would sort first in that group if the disabled flag were ignored; the + // last_active value keeps it in the intended group (offline