From e7af84df72dd2ad42925116c51a76ec0f820caf1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kalash=20Thakare=20=E2=98=AF=EF=B8=8E?= Date: Sun, 17 May 2026 12:11:39 +0530 Subject: [PATCH] feat: execute post run cleanup when workflow is cancelled (#37275) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Fixes #36983 ## Summary 1. Add transitional `Cancelling` status (between `Running` and `Cancelled`); cancel flow marks active tasks `Cancelling`, runner finalizes to `Cancelled` on terminal result. 2. Taskless jobs cancel directly (no runner to finalize). 3. Runner-protocol responses map `Cancelling` → `RESULT_CANCELLED`. 4. Run/job aggregation treats `Cancelling` as active. 5. Status mapping/aggregation tests + en-US locale added. **Problem** When a workflow was cancelled from the UI, jobs were marked cancelled immediately, which could skip post-run cleanup behavior. ## Solution Use a transitional status path: Running → Cancelling → Cancelled This allows runner finalization and cleanup path execution before final terminal state. **Testing** > 1. go test -tags "sqlite sqlite_unlock_notify" ./models/actions -run "TestAggregateJobStatus|TestStatusAsResult|TestStatusFromResult" > 2. go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.11.4 run ./models/actions/... ./routers/api/actions/runner/... ## Related - act_runner: https://gitea.com/gitea/act_runner/pulls/825 — independent; this PR's capability gate keeps legacy runners on the immediate-cancel path. The new flow activates only for runners that advertise the `cancelling` capability. Co-authored-by: Nicolas Co-authored-by: silverwind Co-authored-by: Claude (Opus 4.7) Co-authored-by: Zettat123 Co-authored-by: Giteabot --- models/actions/main_test.go | 1 + models/actions/run.go | 5 +- models/actions/run_job.go | 21 +- models/actions/run_job_status_test.go | 19 +- models/actions/run_list.go | 4 +- models/actions/runner.go | 2 + models/actions/status.go | 67 +++-- models/actions/status_test.go | 49 ++++ models/actions/task.go | 40 ++- models/actions/task_test.go | 231 ++++++++++++++++++ models/migrations/migrations.go | 1 + models/migrations/v1_27/v334.go | 17 ++ models/migrations/v1_27/v334_test.go | 36 +++ options/locale/locale_en-US.json | 1 + routers/api/actions/runner/runner.go | 63 ++++- routers/api/actions/runner/runner_test.go | 86 +++++++ routers/web/repo/actions/view.go | 19 +- routers/web/repo/actions/view_test.go | 31 +++ services/actions/cleanup.go | 2 +- services/actions/clear_tasks.go | 72 +++--- services/actions/clear_tasks_test.go | 90 +++++++ services/actions/commit_status.go | 6 +- services/actions/commit_status_test.go | 3 +- services/actions/notify.go | 13 +- services/actions/task.go | 2 +- templates/repo/actions/view_component.tmpl | 1 + templates/repo/icons/action_status.tmpl | 4 +- tests/integration/actions_concurrency_test.go | 3 + web_src/js/components/ActionStatusIcon.vue | 5 +- web_src/js/features/repo-actions.ts | 1 + web_src/js/modules/gitea-actions.ts | 2 +- 31 files changed, 786 insertions(+), 111 deletions(-) create mode 100644 models/actions/status_test.go create mode 100644 models/migrations/v1_27/v334.go create mode 100644 models/migrations/v1_27/v334_test.go create mode 100644 routers/api/actions/runner/runner_test.go create mode 100644 services/actions/clear_tasks_test.go diff --git a/models/actions/main_test.go b/models/actions/main_test.go index 4af483813a..3c878c6844 100644 --- a/models/actions/main_test.go +++ b/models/actions/main_test.go @@ -15,6 +15,7 @@ func TestMain(m *testing.M) { "action_runner_token.yml", "action_run.yml", "repository.yml", + "user.yml", }, }) } diff --git a/models/actions/run.go b/models/actions/run.go index 5d7f51ade3..61cc4599b6 100644 --- a/models/actions/run.go +++ b/models/actions/run.go @@ -262,7 +262,7 @@ func CancelPreviousJobs(ctx context.Context, repoID int64, ref, workflowID strin Ref: ref, WorkflowID: workflowID, TriggerEvent: event, - Status: []Status{StatusRunning, StatusWaiting, StatusBlocked}, + Status: []Status{StatusRunning, StatusWaiting, StatusBlocked, StatusCancelling}, }) if err != nil { return nil, err @@ -329,7 +329,7 @@ func CancelJobs(ctx context.Context, jobs []*ActionRunJob) ([]*ActionRunJob, err } // If the job has an associated task, try to stop the task, effectively cancelling the job. - if err := StopTask(ctx, job.TaskID, StatusCancelled); err != nil { + if err := StopTask(ctx, job.TaskID, StatusCancelling); err != nil { return cancelledJobs, err } updatedJob, err := GetRunJobByRunAndID(ctx, job.RunID, job.ID) @@ -452,6 +452,7 @@ func CancelPreviousJobsByRunConcurrency(ctx context.Context, attempt *ActionRunA statusFindOption := []Status{StatusWaiting, StatusBlocked} if attempt.ConcurrencyCancel { statusFindOption = append(statusFindOption, StatusRunning) + statusFindOption = append(statusFindOption, StatusCancelling) } attempts, jobs, err := GetConcurrentRunAttemptsAndJobs(ctx, attempt.RepoID, attempt.ConcurrencyGroup, statusFindOption) if err != nil { diff --git a/models/actions/run_job.go b/models/actions/run_job.go index f0d41ef4b4..7a63845401 100644 --- a/models/actions/run_job.go +++ b/models/actions/run_job.go @@ -235,7 +235,10 @@ func UpdateRunJob(ctx context.Context, job *ActionRunJob, cond builder.Cond, col return 0, err } - if affected == 0 || (!slices.Contains(cols, "status") && job.Status == 0) { + // xorm's Update writes only non-zero fields when cols is empty, so a zero job.Status + // with empty cols means status isn't actually being persisted — skip aggregation. + statusUpdated := slices.Contains(cols, "status") || (len(cols) == 0 && job.Status != 0) + if affected == 0 || !statusUpdated { return affected, nil } @@ -308,12 +311,13 @@ func UpdateRunJob(ctx context.Context, job *ActionRunJob, cond builder.Cond, col func AggregateJobStatus(jobs []*ActionRunJob) Status { allSuccessOrSkipped := len(jobs) != 0 allSkipped := len(jobs) != 0 - var hasFailure, hasCancelled, hasWaiting, hasRunning, hasBlocked bool + var hasFailure, hasCancelled, hasCancelling, hasWaiting, hasRunning, hasBlocked bool for _, job := range jobs { allSuccessOrSkipped = allSuccessOrSkipped && (job.Status == StatusSuccess || job.Status == StatusSkipped) allSkipped = allSkipped && job.Status == StatusSkipped hasFailure = hasFailure || job.Status == StatusFailure hasCancelled = hasCancelled || job.Status == StatusCancelled + hasCancelling = hasCancelling || job.Status == StatusCancelling hasWaiting = hasWaiting || job.Status == StatusWaiting hasRunning = hasRunning || job.Status == StatusRunning hasBlocked = hasBlocked || job.Status == StatusBlocked @@ -323,16 +327,20 @@ func AggregateJobStatus(jobs []*ActionRunJob) Status { return StatusSkipped case allSuccessOrSkipped: return StatusSuccess - case hasCancelled: - return StatusCancelled + case hasCancelling: + return StatusCancelling case hasRunning: return StatusRunning case hasWaiting: return StatusWaiting + case hasBlocked: + // Blocked is still a pending state, so it should outrank terminal + // statuses like cancelled/failure when no job is waiting or running. + return StatusBlocked + case hasCancelled: + return StatusCancelled case hasFailure: return StatusFailure - case hasBlocked: - return StatusBlocked default: return StatusUnknown // it shouldn't happen } @@ -352,6 +360,7 @@ func CancelPreviousJobsByJobConcurrency(ctx context.Context, job *ActionRunJob) statusFindOption := []Status{StatusWaiting, StatusBlocked} if job.ConcurrencyCancel { statusFindOption = append(statusFindOption, StatusRunning) + statusFindOption = append(statusFindOption, StatusCancelling) } attempts, jobs, err := GetConcurrentRunAttemptsAndJobs(ctx, job.RepoID, job.ConcurrencyGroup, statusFindOption) if err != nil { diff --git a/models/actions/run_job_status_test.go b/models/actions/run_job_status_test.go index b9ae9f34bf..c1a44cc6d3 100644 --- a/models/actions/run_job_status_test.go +++ b/models/actions/run_job_status_test.go @@ -36,6 +36,7 @@ func TestAggregateJobStatus(t *testing.T) { {[]Status{StatusUnknown, StatusSkipped}, StatusUnknown}, {[]Status{StatusUnknown, StatusFailure}, StatusFailure}, {[]Status{StatusUnknown, StatusCancelled}, StatusCancelled}, + {[]Status{StatusUnknown, StatusCancelling}, StatusCancelling}, {[]Status{StatusUnknown, StatusWaiting}, StatusWaiting}, {[]Status{StatusUnknown, StatusRunning}, StatusRunning}, {[]Status{StatusUnknown, StatusBlocked}, StatusBlocked}, @@ -45,6 +46,7 @@ func TestAggregateJobStatus(t *testing.T) { {[]Status{StatusSuccess, StatusSkipped}, StatusSuccess}, // skipped doesn't affect success {[]Status{StatusSuccess, StatusFailure}, StatusFailure}, {[]Status{StatusSuccess, StatusCancelled}, StatusCancelled}, + {[]Status{StatusSuccess, StatusCancelling}, StatusCancelling}, {[]Status{StatusSuccess, StatusWaiting}, StatusWaiting}, {[]Status{StatusSuccess, StatusRunning}, StatusRunning}, {[]Status{StatusSuccess, StatusBlocked}, StatusBlocked}, @@ -54,9 +56,16 @@ func TestAggregateJobStatus(t *testing.T) { {[]Status{StatusCancelled, StatusSuccess}, StatusCancelled}, {[]Status{StatusCancelled, StatusSkipped}, StatusCancelled}, {[]Status{StatusCancelled, StatusFailure}, StatusCancelled}, - {[]Status{StatusCancelled, StatusWaiting}, StatusCancelled}, - {[]Status{StatusCancelled, StatusRunning}, StatusCancelled}, - {[]Status{StatusCancelled, StatusBlocked}, StatusCancelled}, + {[]Status{StatusCancelled, StatusCancelling}, StatusCancelling}, + {[]Status{StatusCancelled, StatusWaiting}, StatusWaiting}, + {[]Status{StatusCancelled, StatusRunning}, StatusRunning}, + {[]Status{StatusCancelled, StatusBlocked}, StatusBlocked}, + + {[]Status{StatusCancelling}, StatusCancelling}, + {[]Status{StatusCancelling, StatusRunning}, StatusCancelling}, + {[]Status{StatusCancelling, StatusWaiting}, StatusCancelling}, + {[]Status{StatusCancelling, StatusFailure}, StatusCancelling}, + {[]Status{StatusCancelling, StatusSkipped}, StatusCancelling}, // failure with other status, usually fail fast, but "running" wins to match GitHub's behavior // another reason that we can't make "failure" wins over "running": it would cause a weird behavior that user cannot cancel a workflow or get current running workflows correctly by filter after a job fail. @@ -64,9 +73,10 @@ func TestAggregateJobStatus(t *testing.T) { {[]Status{StatusFailure, StatusSuccess}, StatusFailure}, {[]Status{StatusFailure, StatusSkipped}, StatusFailure}, {[]Status{StatusFailure, StatusCancelled}, StatusCancelled}, + {[]Status{StatusFailure, StatusCancelling}, StatusCancelling}, {[]Status{StatusFailure, StatusWaiting}, StatusWaiting}, {[]Status{StatusFailure, StatusRunning}, StatusRunning}, - {[]Status{StatusFailure, StatusBlocked}, StatusFailure}, + {[]Status{StatusFailure, StatusBlocked}, StatusBlocked}, // skipped with other status // "all skipped" is also considered as "mergeable" by "services/actions.toCommitStatus", the same as GitHub @@ -74,6 +84,7 @@ func TestAggregateJobStatus(t *testing.T) { {[]Status{StatusSkipped, StatusSuccess}, StatusSuccess}, {[]Status{StatusSkipped, StatusFailure}, StatusFailure}, {[]Status{StatusSkipped, StatusCancelled}, StatusCancelled}, + {[]Status{StatusSkipped, StatusCancelling}, StatusCancelling}, {[]Status{StatusSkipped, StatusWaiting}, StatusWaiting}, {[]Status{StatusSkipped, StatusRunning}, StatusRunning}, {[]Status{StatusSkipped, StatusBlocked}, StatusBlocked}, diff --git a/models/actions/run_list.go b/models/actions/run_list.go index 0a0840648d..bfa9bbfb16 100644 --- a/models/actions/run_list.go +++ b/models/actions/run_list.go @@ -121,8 +121,8 @@ type StatusInfo struct { // GetStatusInfoList returns a slice of StatusInfo func GetStatusInfoList(ctx context.Context, lang translation.Locale) []StatusInfo { // same as those in aggregateJobStatus - allStatus := []Status{StatusSuccess, StatusFailure, StatusWaiting, StatusRunning} - statusInfoList := make([]StatusInfo, 0, 4) + allStatus := []Status{StatusSuccess, StatusFailure, StatusWaiting, StatusRunning, StatusCancelling} + statusInfoList := make([]StatusInfo, 0, len(allStatus)) for _, s := range allStatus { statusInfoList = append(statusInfoList, StatusInfo{ Status: int(s), diff --git a/models/actions/runner.go b/models/actions/runner.go index f0088491bb..27b7509ac4 100644 --- a/models/actions/runner.go +++ b/models/actions/runner.go @@ -64,6 +64,8 @@ type ActionRunner struct { Ephemeral bool `xorm:"ephemeral NOT NULL DEFAULT false"` // Store if this runner is disabled and should not pick up new jobs IsDisabled bool `xorm:"is_disabled NOT NULL DEFAULT false"` + // Store if this runner supports the StatusCancelling flow + HasCancellingSupport bool `xorm:"has_cancelling_support NOT NULL DEFAULT false"` Created timeutil.TimeStamp `xorm:"created"` Updated timeutil.TimeStamp `xorm:"updated"` diff --git a/models/actions/status.go b/models/actions/status.go index 2b1d70613c..f0701ad3b6 100644 --- a/models/actions/status.go +++ b/models/actions/status.go @@ -15,25 +15,27 @@ import ( type Status int const ( - StatusUnknown Status = iota // 0, consistent with runnerv1.Result_RESULT_UNSPECIFIED - StatusSuccess // 1, consistent with runnerv1.Result_RESULT_SUCCESS - StatusFailure // 2, consistent with runnerv1.Result_RESULT_FAILURE - StatusCancelled // 3, consistent with runnerv1.Result_RESULT_CANCELLED - StatusSkipped // 4, consistent with runnerv1.Result_RESULT_SKIPPED - StatusWaiting // 5, isn't a runnerv1.Result - StatusRunning // 6, isn't a runnerv1.Result - StatusBlocked // 7, isn't a runnerv1.Result + StatusUnknown Status = iota // 0, consistent with runnerv1.Result_RESULT_UNSPECIFIED + StatusSuccess // 1, consistent with runnerv1.Result_RESULT_SUCCESS + StatusFailure // 2, consistent with runnerv1.Result_RESULT_FAILURE + StatusCancelled // 3, consistent with runnerv1.Result_RESULT_CANCELLED + StatusSkipped // 4, consistent with runnerv1.Result_RESULT_SKIPPED + StatusWaiting // 5, isn't a runnerv1.Result + StatusRunning // 6, isn't a runnerv1.Result + StatusBlocked // 7, isn't a runnerv1.Result + StatusCancelling // 8, isn't a runnerv1.Result ) var statusNames = map[Status]string{ - StatusUnknown: "unknown", - StatusWaiting: "waiting", - StatusRunning: "running", - StatusSuccess: "success", - StatusFailure: "failure", - StatusCancelled: "cancelled", - StatusSkipped: "skipped", - StatusBlocked: "blocked", + StatusUnknown: "unknown", + StatusWaiting: "waiting", + StatusRunning: "running", + StatusSuccess: "success", + StatusFailure: "failure", + StatusCancelled: "cancelled", + StatusCancelling: "cancelling", + StatusSkipped: "skipped", + StatusBlocked: "blocked", } // String returns the string name of the Status @@ -88,14 +90,41 @@ func (s Status) IsBlocked() bool { return s == StatusBlocked } +func (s Status) IsCancelling() bool { + return s == StatusCancelling +} + // In returns whether s is one of the given statuses func (s Status) In(statuses ...Status) bool { return slices.Contains(statuses, s) } func (s Status) AsResult() runnerv1.Result { - if s.IsDone() { - return runnerv1.Result(s) + switch s { + case StatusSuccess: + return runnerv1.Result_RESULT_SUCCESS + case StatusFailure: + return runnerv1.Result_RESULT_FAILURE + case StatusCancelled, StatusCancelling: + return runnerv1.Result_RESULT_CANCELLED + case StatusSkipped: + return runnerv1.Result_RESULT_SKIPPED + default: + return runnerv1.Result_RESULT_UNSPECIFIED + } +} + +func StatusFromResult(r runnerv1.Result) Status { + switch r { + case runnerv1.Result_RESULT_SUCCESS: + return StatusSuccess + case runnerv1.Result_RESULT_FAILURE: + return StatusFailure + case runnerv1.Result_RESULT_CANCELLED: + return StatusCancelled + case runnerv1.Result_RESULT_SKIPPED: + return StatusSkipped + default: + return StatusUnknown } - return runnerv1.Result_RESULT_UNSPECIFIED } diff --git a/models/actions/status_test.go b/models/actions/status_test.go new file mode 100644 index 0000000000..2363101c5d --- /dev/null +++ b/models/actions/status_test.go @@ -0,0 +1,49 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package actions + +import ( + "testing" + + runnerv1 "code.gitea.io/actions-proto-go/runner/v1" + "github.com/stretchr/testify/assert" +) + +func TestStatusAsResult(t *testing.T) { + cases := []struct { + status Status + want runnerv1.Result + }{ + {StatusUnknown, runnerv1.Result_RESULT_UNSPECIFIED}, + {StatusWaiting, runnerv1.Result_RESULT_UNSPECIFIED}, + {StatusRunning, runnerv1.Result_RESULT_UNSPECIFIED}, + {StatusBlocked, runnerv1.Result_RESULT_UNSPECIFIED}, + {StatusSuccess, runnerv1.Result_RESULT_SUCCESS}, + {StatusFailure, runnerv1.Result_RESULT_FAILURE}, + {StatusCancelled, runnerv1.Result_RESULT_CANCELLED}, + {StatusCancelling, runnerv1.Result_RESULT_CANCELLED}, + {StatusSkipped, runnerv1.Result_RESULT_SKIPPED}, + } + + for _, tt := range cases { + assert.Equal(t, tt.want, tt.status.AsResult(), "status=%s", tt.status) + } +} + +func TestStatusFromResult(t *testing.T) { + cases := []struct { + result runnerv1.Result + want Status + }{ + {runnerv1.Result_RESULT_UNSPECIFIED, StatusUnknown}, + {runnerv1.Result_RESULT_SUCCESS, StatusSuccess}, + {runnerv1.Result_RESULT_FAILURE, StatusFailure}, + {runnerv1.Result_RESULT_CANCELLED, StatusCancelled}, + {runnerv1.Result_RESULT_SKIPPED, StatusSkipped}, + } + + for _, tt := range cases { + assert.Equal(t, tt.want, StatusFromResult(tt.result), "result=%s", tt.result) + } +} diff --git a/models/actions/task.go b/models/actions/task.go index 7a97eadc79..38a92714b0 100644 --- a/models/actions/task.go +++ b/models/actions/task.go @@ -193,7 +193,8 @@ func GetRunningTaskByToken(ctx context.Context, token string) (*ActionTask, erro } var tasks []*ActionTask - err := db.GetEngine(ctx).Where("token_last_eight = ? AND status = ?", lastEight, StatusRunning).Find(&tasks) + // Cancelling tasks are still authenticating — post-run cleanup steps need API access (artifact uploads, cache saves, etc.) before the runner finalizes the task. + err := db.GetEngine(ctx).Where("token_last_eight = ? AND status IN (?, ?)", lastEight, StatusRunning, StatusCancelling).Find(&tasks) if err != nil { return nil, err } else if len(tasks) == 0 { @@ -374,7 +375,12 @@ func UpdateTaskByState(ctx context.Context, runnerID int64, state *runnerv1.Task // state.Result is not unspecified means the task is finished if state.Result != runnerv1.Result_RESULT_UNSPECIFIED { - task.Status = Status(state.Result) + if task.Status == StatusCancelling { + // The runner may report SUCCESS/FAILURE for the cleanup phase; preserve user intent. + task.Status = StatusCancelled + } else { + task.Status = StatusFromResult(state.Result) + } task.Stopped = timeutil.TimeStamp(state.StoppedAt.AsTime().Unix()) if err := UpdateTask(ctx, task, "status", "stopped"); err != nil { return nil, err @@ -409,7 +415,7 @@ func UpdateTaskByState(ctx context.Context, runnerID int64, state *runnerv1.Task step.Stopped = convertTimestamp(v.StoppedAt) } if result != runnerv1.Result_RESULT_UNSPECIFIED { - step.Status = Status(result) + step.Status = StatusFromResult(result) } else if step.Started != 0 { step.Status = StatusRunning } @@ -423,7 +429,7 @@ func UpdateTaskByState(ctx context.Context, runnerID int64, state *runnerv1.Task } func StopTask(ctx context.Context, taskID int64, status Status) error { - if !status.IsDone() { + if !status.IsDone() && status != StatusCancelling { return fmt.Errorf("cannot stop task with status %v", status) } e := db.GetEngine(ctx) @@ -439,6 +445,32 @@ func StopTask(ctx context.Context, taskID int64, status Status) error { } now := timeutil.TimeStampNow() + if status == StatusCancelling { + runner, err := GetRunnerByID(ctx, task.RunnerID) + if err != nil { + if !errors.Is(err, util.ErrNotExist) { + return err + } + status = StatusCancelled + } else if !runner.HasCancellingSupport { + status = StatusCancelled + } + } + + if status == StatusCancelling { + task.Status = StatusCancelling + + if _, err := UpdateRunJob(ctx, &ActionRunJob{ + ID: task.JobID, + RepoID: task.RepoID, + Status: StatusCancelling, + }, nil, "status"); err != nil { + return err + } + + return UpdateTask(ctx, task, "status") + } + task.Status = status task.Stopped = now if _, err := UpdateRunJob(ctx, &ActionRunJob{ diff --git a/models/actions/task_test.go b/models/actions/task_test.go index cbfc6f7636..9d719d3c3a 100644 --- a/models/actions/task_test.go +++ b/models/actions/task_test.go @@ -7,9 +7,15 @@ import ( "strings" "testing" + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/actions/jobparser" + "code.gitea.io/gitea/modules/timeutil" + runnerv1 "code.gitea.io/actions-proto-go/runner/v1" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/types/known/timestamppb" ) func TestMakeTaskStepDisplayName(t *testing.T) { @@ -75,3 +81,228 @@ func TestMakeTaskStepDisplayName(t *testing.T) { }) } } + +func TestTaskCancellingFinalizesToCancelled(t *testing.T) { + newRunningTask := func(t *testing.T) (*ActionTask, *ActionRunJob) { + t.Helper() + + run := &ActionRun{ + Title: "cancelling-test-run", + RepoID: 1, + OwnerID: 2, + WorkflowID: "test.yaml", + Index: 999, + TriggerUserID: 2, + Ref: "refs/heads/master", + CommitSHA: "c2d72f548424103f01ee1dc02889c1e2bff816b0", + Event: "push", + TriggerEvent: "push", + Status: StatusRunning, + Started: timeutil.TimeStampNow(), + } + require.NoError(t, db.Insert(t.Context(), run)) + + job := &ActionRunJob{ + RunID: run.ID, + RepoID: run.RepoID, + OwnerID: run.OwnerID, + CommitSHA: run.CommitSHA, + Name: "cancelling-finalization-job", + Attempt: 1, + JobID: "cancelling-finalization-job", + Status: StatusRunning, + } + require.NoError(t, db.Insert(t.Context(), job)) + + runner := &ActionRunner{ + UUID: "runner-cancelling-supported", + Name: "runner-cancelling-supported", + HasCancellingSupport: true, + } + require.NoError(t, db.Insert(t.Context(), runner)) + + task := &ActionTask{ + JobID: job.ID, + Attempt: 1, + RunnerID: runner.ID, + Status: StatusRunning, + Started: timeutil.TimeStampNow(), + RepoID: run.RepoID, + OwnerID: run.OwnerID, + CommitSHA: run.CommitSHA, + } + require.NoError(t, db.Insert(t.Context(), task)) + + job.TaskID = task.ID + _, err := UpdateRunJob(t.Context(), job, nil, "task_id") + require.NoError(t, err) + + return task, job + } + + testResult := func(t *testing.T, result runnerv1.Result) { + t.Helper() + require.NoError(t, unittest.PrepareTestDatabase()) + + task, job := newRunningTask(t) + require.NoError(t, StopTask(t.Context(), task.ID, StatusCancelling)) + + taskAfterStop := unittest.AssertExistsAndLoadBean(t, &ActionTask{ID: task.ID}) + assert.Equal(t, StatusCancelling, taskAfterStop.Status) + + updatedTask, err := UpdateTaskByState(t.Context(), task.RunnerID, &runnerv1.TaskState{ + Id: task.ID, + Result: result, + StoppedAt: timestamppb.Now(), + }) + require.NoError(t, err) + assert.Equal(t, StatusCancelled, updatedTask.Status) + + taskAfterUpdate := unittest.AssertExistsAndLoadBean(t, &ActionTask{ID: task.ID}) + assert.Equal(t, StatusCancelled, taskAfterUpdate.Status) + + jobAfterUpdate := unittest.AssertExistsAndLoadBean(t, &ActionRunJob{ID: job.ID}) + assert.Equal(t, StatusCancelled, jobAfterUpdate.Status) + } + + t.Run("runner reports success", func(t *testing.T) { + testResult(t, runnerv1.Result_RESULT_SUCCESS) + }) + + t.Run("runner reports failure", func(t *testing.T) { + testResult(t, runnerv1.Result_RESULT_FAILURE) + }) +} + +func TestStopTaskCancellingFallsBackForLegacyRunner(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + run := &ActionRun{ + Title: "cancelling-test-run", + RepoID: 1, + OwnerID: 2, + WorkflowID: "test.yaml", + Index: 999, + TriggerUserID: 2, + Ref: "refs/heads/master", + CommitSHA: "c2d72f548424103f01ee1dc02889c1e2bff816b0", + Event: "push", + TriggerEvent: "push", + Status: StatusRunning, + Started: timeutil.TimeStampNow(), + } + require.NoError(t, db.Insert(t.Context(), run)) + + job := &ActionRunJob{ + RunID: run.ID, + RepoID: run.RepoID, + OwnerID: run.OwnerID, + CommitSHA: run.CommitSHA, + Name: "legacy-cancelling-job", + Attempt: 1, + JobID: "legacy-cancelling-job", + Status: StatusRunning, + } + require.NoError(t, db.Insert(t.Context(), job)) + + runner := &ActionRunner{ + UUID: "runner-legacy-no-cancelling", + Name: "runner-legacy-no-cancelling", + HasCancellingSupport: false, + } + require.NoError(t, db.Insert(t.Context(), runner)) + + task := &ActionTask{ + JobID: job.ID, + Attempt: 1, + RunnerID: runner.ID, + Status: StatusRunning, + Started: timeutil.TimeStampNow(), + RepoID: run.RepoID, + OwnerID: run.OwnerID, + CommitSHA: run.CommitSHA, + } + require.NoError(t, db.Insert(t.Context(), task)) + + job.TaskID = task.ID + _, err := UpdateRunJob(t.Context(), job, nil, "task_id") + require.NoError(t, err) + + require.NoError(t, StopTask(t.Context(), task.ID, StatusCancelling)) + + taskAfterStop := unittest.AssertExistsAndLoadBean(t, &ActionTask{ID: task.ID}) + assert.Equal(t, StatusCancelled, taskAfterStop.Status) + assert.NotZero(t, taskAfterStop.Stopped) + + jobAfterStop := unittest.AssertExistsAndLoadBean(t, &ActionRunJob{ID: job.ID}) + assert.Equal(t, StatusCancelled, jobAfterStop.Status) + assert.NotZero(t, jobAfterStop.Stopped) +} + +func TestStopTaskCancellingFallsBackForMissingRunner(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + run := &ActionRun{ + Title: "cancelling-test-run", + RepoID: 1, + OwnerID: 2, + WorkflowID: "test.yaml", + Index: 999, + TriggerUserID: 2, + Ref: "refs/heads/master", + CommitSHA: "c2d72f548424103f01ee1dc02889c1e2bff816b0", + Event: "push", + TriggerEvent: "push", + Status: StatusRunning, + Started: timeutil.TimeStampNow(), + } + require.NoError(t, db.Insert(t.Context(), run)) + + job := &ActionRunJob{ + RunID: run.ID, + RepoID: run.RepoID, + OwnerID: run.OwnerID, + CommitSHA: run.CommitSHA, + Name: "missing-runner-cancelling-job", + Attempt: 1, + JobID: "missing-runner-cancelling-job", + Status: StatusRunning, + } + require.NoError(t, db.Insert(t.Context(), job)) + + runner := &ActionRunner{ + UUID: "runner-cleaned-up-before-cancel", + Name: "runner-cleaned-up-before-cancel", + HasCancellingSupport: true, + } + require.NoError(t, db.Insert(t.Context(), runner)) + + task := &ActionTask{ + JobID: job.ID, + Attempt: 1, + RunnerID: runner.ID, + Status: StatusRunning, + Started: timeutil.TimeStampNow(), + RepoID: run.RepoID, + OwnerID: run.OwnerID, + CommitSHA: run.CommitSHA, + } + require.NoError(t, db.Insert(t.Context(), task)) + + job.TaskID = task.ID + _, err := UpdateRunJob(t.Context(), job, nil, "task_id") + require.NoError(t, err) + + _, err = db.DeleteByID[ActionRunner](t.Context(), runner.ID) + require.NoError(t, err) + + require.NoError(t, StopTask(t.Context(), task.ID, StatusCancelling)) + + taskAfterStop := unittest.AssertExistsAndLoadBean(t, &ActionTask{ID: task.ID}) + assert.Equal(t, StatusCancelled, taskAfterStop.Status) + assert.NotZero(t, taskAfterStop.Stopped) + + jobAfterStop := unittest.AssertExistsAndLoadBean(t, &ActionRunJob{ID: job.ID}) + assert.Equal(t, StatusCancelled, jobAfterStop.Status) + assert.NotZero(t, jobAfterStop.Stopped) +} diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 3689f448d8..65a950fc9c 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -411,6 +411,7 @@ func prepareMigrationTasks() []*migration { newMigration(331, "Add ActionRunAttempt model and related action fields", v1_27.AddActionRunAttemptModel), newMigration(332, "Add last_sync_unix to mirror", v1_27.AddLastSyncUnixToMirror), newMigration(333, "Add bypass allowlist to branch protection", v1_27.AddBranchProtectionBypassAllowlist), + newMigration(334, "Add cancelling support to action runners", v1_27.AddCancellingSupportToActionRunner), } return preparedMigrations } diff --git a/models/migrations/v1_27/v334.go b/models/migrations/v1_27/v334.go new file mode 100644 index 0000000000..02237a70ae --- /dev/null +++ b/models/migrations/v1_27/v334.go @@ -0,0 +1,17 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package v1_27 + +import "xorm.io/xorm" + +func AddCancellingSupportToActionRunner(x *xorm.Engine) error { + type ActionRunner struct { + HasCancellingSupport bool `xorm:"has_cancelling_support NOT NULL DEFAULT false"` + } + + _, err := x.SyncWithOptions(xorm.SyncOptions{ + IgnoreDropIndices: true, + }, new(ActionRunner)) + return err +} diff --git a/models/migrations/v1_27/v334_test.go b/models/migrations/v1_27/v334_test.go new file mode 100644 index 0000000000..29369d5332 --- /dev/null +++ b/models/migrations/v1_27/v334_test.go @@ -0,0 +1,36 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package v1_27 + +import ( + "testing" + + "code.gitea.io/gitea/models/migrations/migrationtest" + + "github.com/stretchr/testify/require" +) + +func TestAddCancellingSupportToActionRunner(t *testing.T) { + type ActionRunner struct { + ID int64 `xorm:"pk autoincr"` + Name string + } + + x, deferable := migrationtest.PrepareTestEnv(t, 0, new(ActionRunner)) + defer deferable() + if x == nil || t.Failed() { + return + } + + _, err := x.Insert(&ActionRunner{Name: "runner"}) + require.NoError(t, err) + + require.NoError(t, AddCancellingSupportToActionRunner(x)) + + var hasCancellingSupport bool + has, err := x.SQL("SELECT has_cancelling_support FROM action_runner WHERE id = ?", 1).Get(&hasCancellingSupport) + require.NoError(t, err) + require.True(t, has) + require.False(t, hasCancellingSupport) +} diff --git a/options/locale/locale_en-US.json b/options/locale/locale_en-US.json index 46992c5760..1abfe647c8 100644 --- a/options/locale/locale_en-US.json +++ b/options/locale/locale_en-US.json @@ -3710,6 +3710,7 @@ "actions.status.success": "Success", "actions.status.failure": "Failure", "actions.status.cancelled": "Canceled", + "actions.status.cancelling": "Canceling", "actions.status.skipped": "Skipped", "actions.status.blocked": "Blocked", "actions.runners": "Runners", diff --git a/routers/api/actions/runner/runner.go b/routers/api/actions/runner/runner.go index 0c9c2a5f4a..0dee1db717 100644 --- a/routers/api/actions/runner/runner.go +++ b/routers/api/actions/runner/runner.go @@ -7,6 +7,7 @@ import ( "context" "errors" "net/http" + "slices" actions_model "code.gitea.io/gitea/models/actions" repo_model "code.gitea.io/gitea/models/repo" @@ -22,6 +23,7 @@ import ( gouuid "github.com/google/uuid" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" + "google.golang.org/protobuf/proto" ) func NewRunnerServiceHandler() (string, http.Handler) { @@ -67,17 +69,19 @@ func (s *Service) Register( } labels := req.Msg.Labels + hasCancellingSupport, _ := runnerRequestHasCancellingCapability(req.Msg) // create new runner name := util.EllipsisDisplayString(req.Msg.Name, 255) runner := &actions_model.ActionRunner{ - UUID: gouuid.New().String(), - Name: name, - OwnerID: runnerToken.OwnerID, - RepoID: runnerToken.RepoID, - Version: req.Msg.Version, - AgentLabels: labels, - Ephemeral: req.Msg.Ephemeral, + UUID: gouuid.New().String(), + Name: name, + OwnerID: runnerToken.OwnerID, + RepoID: runnerToken.RepoID, + Version: req.Msg.Version, + AgentLabels: labels, + Ephemeral: req.Msg.Ephemeral, + HasCancellingSupport: hasCancellingSupport, } runner.GenerateAndFillToken() @@ -107,14 +111,53 @@ func (s *Service) Register( return res, nil } +// runnerCapabilityCancelling is the wire string the runner advertises in its +// capabilities list to indicate it understands the transitional cancelling +// state and will run post-step cleanup before finalizing the task. +const runnerCapabilityCancelling = "cancelling" + +type capabilityGetter interface { + GetCapabilities() []string +} + +type declareRequest interface { + proto.Message + GetVersion() string + GetLabels() []string +} + +func runnerRequestHasCancellingCapability(req proto.Message) (bool, bool) { + if req == nil { + return false, false + } + + if typedReq, ok := any(req).(capabilityGetter); ok { + return slices.Contains(typedReq.GetCapabilities(), runnerCapabilityCancelling), true + } + + return false, false +} + +func applyDeclareRequestToRunner(runner *actions_model.ActionRunner, req declareRequest) []string { + runner.AgentLabels = req.GetLabels() + runner.Version = req.GetVersion() + + cols := []string{"agent_labels", "version"} + hasCancellingSupport, capabilityStateKnown := runnerRequestHasCancellingCapability(req) + if capabilityStateKnown && runner.HasCancellingSupport != hasCancellingSupport { + runner.HasCancellingSupport = hasCancellingSupport + cols = append(cols, "has_cancelling_support") + } + + return cols +} + func (s *Service) Declare( ctx context.Context, req *connect.Request[runnerv1.DeclareRequest], ) (*connect.Response[runnerv1.DeclareResponse], error) { runner := GetRunner(ctx) - runner.AgentLabels = req.Msg.Labels - runner.Version = req.Msg.Version - if err := actions_model.UpdateRunner(ctx, runner, "agent_labels", "version"); err != nil { + if err := actions_model.UpdateRunner(ctx, runner, applyDeclareRequestToRunner(runner, req.Msg)...); err != nil { return nil, status.Errorf(codes.Internal, "update runner: %v", err) } diff --git a/routers/api/actions/runner/runner_test.go b/routers/api/actions/runner/runner_test.go new file mode 100644 index 0000000000..dc07343067 --- /dev/null +++ b/routers/api/actions/runner/runner_test.go @@ -0,0 +1,86 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package runner + +import ( + "testing" + + actions_model "code.gitea.io/gitea/models/actions" + + runnerv1 "code.gitea.io/actions-proto-go/runner/v1" + "github.com/stretchr/testify/assert" +) + +type capabilityRegisterRequest struct { + *runnerv1.RegisterRequest + capabilities []string +} + +func (r *capabilityRegisterRequest) GetCapabilities() []string { + return r.capabilities +} + +type capabilityDeclareRequest struct { + *runnerv1.DeclareRequest + capabilities []string +} + +func (r *capabilityDeclareRequest) GetCapabilities() []string { + return r.capabilities +} + +func TestRunnerRequestHasCancellingCapabilityTypedAccessor(t *testing.T) { + registerReq := &capabilityRegisterRequest{ + RegisterRequest: &runnerv1.RegisterRequest{}, + capabilities: []string{runnerCapabilityCancelling, "other"}, + } + hasCapability, known := runnerRequestHasCancellingCapability(registerReq) + assert.True(t, hasCapability) + assert.True(t, known) + + declareReq := &capabilityDeclareRequest{ + DeclareRequest: &runnerv1.DeclareRequest{}, + capabilities: nil, + } + hasCapability, known = runnerRequestHasCancellingCapability(declareReq) + assert.False(t, hasCapability) + assert.True(t, known) + + hasCapability, known = runnerRequestHasCancellingCapability(nil) + assert.False(t, hasCapability) + assert.False(t, known) +} + +func TestApplyDeclareRequestToRunnerPreservesUnknownCapabilityState(t *testing.T) { + runner := &actions_model.ActionRunner{ + HasCancellingSupport: true, + } + req := &runnerv1.DeclareRequest{ + Version: "1.2.3", + Labels: []string{"linux"}, + } + + cols := applyDeclareRequestToRunner(runner, req) + assert.Equal(t, []string{"agent_labels", "version"}, cols) + assert.True(t, runner.HasCancellingSupport) + assert.Equal(t, "1.2.3", runner.Version) + assert.Equal(t, []string{"linux"}, runner.AgentLabels) +} + +func TestApplyDeclareRequestToRunnerUpdatesTypedCapabilityState(t *testing.T) { + runner := &actions_model.ActionRunner{ + HasCancellingSupport: true, + } + req := &capabilityDeclareRequest{ + DeclareRequest: &runnerv1.DeclareRequest{ + Version: "1.2.3", + Labels: []string{"linux"}, + }, + capabilities: []string{}, + } + + cols := applyDeclareRequestToRunner(runner, req) + assert.Equal(t, []string{"agent_labels", "version", "has_cancelling_support"}, cols) + assert.False(t, runner.HasCancellingSupport) +} diff --git a/routers/web/repo/actions/view.go b/routers/web/repo/actions/view.go index 039aa44ec0..2e6efc691f 100644 --- a/routers/web/repo/actions/view.go +++ b/routers/web/repo/actions/view.go @@ -404,19 +404,22 @@ func fillViewRunResponseSummary(ctx *context_module.Context, resp *ViewResponse, resp.State.Run.Link = run.Link() resp.State.Run.ViewLink = getRunViewLink(run, attempt) resp.State.Run.Attempts = make([]*ViewRunAttempt, 0) + var effectiveStatus actions_model.Status if attempt != nil { + effectiveStatus = attempt.Status resp.State.Run.RunAttempt = attempt.Attempt - resp.State.Run.Status = attempt.Status.String() - resp.State.Run.Done = attempt.Status.IsDone() resp.State.Run.Duration = attempt.Duration().String() resp.State.Run.TriggeredAt = attempt.Created.AsTime().Unix() } else { - resp.State.Run.Status = run.Status.String() - resp.State.Run.Done = run.Status.IsDone() + effectiveStatus = run.Status resp.State.Run.Duration = run.Duration().String() resp.State.Run.TriggeredAt = run.Created.AsTime().Unix() } - resp.State.Run.CanCancel = isLatestAttempt && !resp.State.Run.Done && ctx.Repo.Permission.CanWrite(unit.TypeActions) + resp.State.Run.Status = effectiveStatus.String() + resp.State.Run.Done = effectiveStatus.IsDone() + + // Hide the Cancel button once a cancel is already in cancelling progress + resp.State.Run.CanCancel = isLatestAttempt && !resp.State.Run.Done && !effectiveStatus.IsCancelling() && ctx.Repo.Permission.CanWrite(unit.TypeActions) resp.State.Run.CanApprove = isLatestAttempt && run.NeedApproval && ctx.Repo.Permission.CanWrite(unit.TypeActions) resp.State.Run.CanRerun = isLatestAttempt && resp.State.Run.Done && ctx.Repo.Permission.CanWrite(unit.TypeActions) resp.State.Run.CanDeleteArtifact = resp.State.Run.Done && ctx.Repo.Permission.CanWrite(unit.TypeActions) @@ -567,10 +570,14 @@ func convertToViewModel(ctx context.Context, locale translation.Locale, cursors steps := actions.FullSteps(task) for _, v := range steps { + status := v.Status + if task.Status == actions_model.StatusCancelling && status.IsRunning() { + status = actions_model.StatusCancelling + } viewJobs = append(viewJobs, &ViewJobStep{ Summary: v.Name, Duration: v.Duration().String(), - Status: v.Status.String(), + Status: status.String(), }) } diff --git a/routers/web/repo/actions/view_test.go b/routers/web/repo/actions/view_test.go index 7296ea6849..f5e44e95cf 100644 --- a/routers/web/repo/actions/view_test.go +++ b/routers/web/repo/actions/view_test.go @@ -45,3 +45,34 @@ func TestConvertToViewModel(t *testing.T) { } assert.Equal(t, expectedViewJobs, viewJobSteps) } + +func TestConvertToViewModelCancellingTaskDoesNotRenderRunningSteps(t *testing.T) { + task := &actions_model.ActionTask{ + Status: actions_model.StatusCancelling, + Steps: []*actions_model.ActionTaskStep{ + {Name: "Run step-name", Index: 0, Status: actions_model.StatusRunning, LogLength: 1}, + }, + } + + viewJobSteps, _, err := convertToViewModel(t.Context(), translation.MockLocale{}, nil, task) + require.NoError(t, err) + + expectedViewJobs := []*ViewJobStep{ + { + Summary: "Set up job", + Duration: "0s", + Status: "success", + }, + { + Summary: "Run step-name", + Duration: "0s", + Status: "cancelling", + }, + { + Summary: "Complete job", + Duration: "0s", + Status: "waiting", + }, + } + assert.Equal(t, expectedViewJobs, viewJobSteps) +} diff --git a/services/actions/cleanup.go b/services/actions/cleanup.go index 5f605cd265..615ee62b02 100644 --- a/services/actions/cleanup.go +++ b/services/actions/cleanup.go @@ -144,7 +144,7 @@ func CleanupEphemeralRunners(ctx context.Context) error { From(builder.Select("*").From("`action_runner`"), "`action_runner`"). // mysql needs this redundant subquery Join("INNER", "`action_task`", "`action_task`.`runner_id` = `action_runner`.`id`"). Where(builder.Eq{"`action_runner`.`ephemeral`": true}). - And(builder.NotIn("`action_task`.`status`", actions_model.StatusWaiting, actions_model.StatusRunning, actions_model.StatusBlocked)) + And(builder.NotIn("`action_task`.`status`", actions_model.StatusWaiting, actions_model.StatusRunning, actions_model.StatusBlocked, actions_model.StatusCancelling)) b := builder.Delete(builder.In("id", subQuery)).From("`action_runner`") res, err := db.GetEngine(ctx).Exec(b) if err != nil { diff --git a/services/actions/clear_tasks.go b/services/actions/clear_tasks.go index 940f1d8454..3f16768aa9 100644 --- a/services/actions/clear_tasks.go +++ b/services/actions/clear_tasks.go @@ -19,20 +19,30 @@ import ( webhook_module "code.gitea.io/gitea/modules/webhook" ) -// StopZombieTasks stops the task which have running status, but haven't been updated for a long time +// StopZombieTasks stops tasks in running/cancelling status that haven't been updated for a long time func StopZombieTasks(ctx context.Context) error { - return stopTasks(ctx, actions_model.FindTaskOptions{ - Status: actions_model.StatusRunning, + return stopTasksByStatuses(ctx, actions_model.FindTaskOptions{ UpdatedBefore: timeutil.TimeStamp(time.Now().Add(-setting.Actions.ZombieTaskTimeout).Unix()), - }) + }, actions_model.StatusRunning, actions_model.StatusCancelling) } -// StopEndlessTasks stops the tasks which have running status and continuous updates, but don't end for a long time +// StopEndlessTasks stops tasks in running/cancelling status with continuous updates that don't end for a long time func StopEndlessTasks(ctx context.Context) error { - return stopTasks(ctx, actions_model.FindTaskOptions{ - Status: actions_model.StatusRunning, + return stopTasksByStatuses(ctx, actions_model.FindTaskOptions{ StartedBefore: timeutil.TimeStamp(time.Now().Add(-setting.Actions.EndlessTaskTimeout).Unix()), - }) + }, actions_model.StatusRunning, actions_model.StatusCancelling) +} + +func stopTasksByStatuses(ctx context.Context, opts actions_model.FindTaskOptions, statuses ...actions_model.Status) error { + for _, status := range statuses { + optsByStatus := opts + optsByStatus.Status = status + if err := stopTasks(ctx, optsByStatus); err != nil { + return err + } + } + + return nil } func CancelPreviousJobs(ctx context.Context, repoID int64, ref, workflowID string, event webhook_module.HookEventType) error { @@ -59,7 +69,7 @@ func shouldBlockJobByConcurrency(ctx context.Context, job *actions_model.ActionR return false, nil } - attempts, jobs, err := actions_model.GetConcurrentRunAttemptsAndJobs(ctx, job.RepoID, job.ConcurrencyGroup, []actions_model.Status{actions_model.StatusRunning}) + attempts, jobs, err := actions_model.GetConcurrentRunAttemptsAndJobs(ctx, job.RepoID, job.ConcurrencyGroup, []actions_model.Status{actions_model.StatusRunning, actions_model.StatusCancelling}) if err != nil { return false, fmt.Errorf("GetConcurrentRunAttemptsAndJobs: %w", err) } @@ -89,7 +99,7 @@ func shouldBlockRunByConcurrency(ctx context.Context, attempt *actions_model.Act return false, nil } - attempts, jobs, err := actions_model.GetConcurrentRunAttemptsAndJobs(ctx, attempt.RepoID, attempt.ConcurrencyGroup, []actions_model.Status{actions_model.StatusRunning}) + attempts, jobs, err := actions_model.GetConcurrentRunAttemptsAndJobs(ctx, attempt.RepoID, attempt.ConcurrencyGroup, []actions_model.Status{actions_model.StatusRunning, actions_model.StatusCancelling}) if err != nil { return false, fmt.Errorf("find concurrent runs and jobs: %w", err) } @@ -123,7 +133,11 @@ func stopTasks(ctx context.Context, opts actions_model.FindTaskOptions) error { jobs := make([]*actions_model.ActionRunJob, 0, len(tasks)) for _, task := range tasks { if err := db.WithTx(ctx, func(ctx context.Context) error { - if err := actions_model.StopTask(ctx, task.ID, actions_model.StatusFailure); err != nil { + stopStatus := actions_model.StatusFailure + if task.Status == actions_model.StatusCancelling { + stopStatus = actions_model.StatusCancelled + } + if err := actions_model.StopTask(ctx, task.ID, stopStatus); err != nil { return err } if err := task.LoadJob(ctx); err != nil { @@ -157,44 +171,18 @@ func stopTasks(ctx context.Context, opts actions_model.FindTaskOptions) error { // CancelAbandonedJobs cancels jobs that have not been picked by any runner for a long time func CancelAbandonedJobs(ctx context.Context) error { - jobs, err := db.Find[actions_model.ActionRunJob](ctx, actions_model.FindRunJobOptions{ + abandonedJobs, err := db.Find[actions_model.ActionRunJob](ctx, actions_model.FindRunJobOptions{ Statuses: []actions_model.Status{actions_model.StatusWaiting, actions_model.StatusBlocked}, UpdatedBefore: timeutil.TimeStampNow().AddDuration(-setting.Actions.AbandonedJobTimeout), }) if err != nil { - log.Warn("find abandoned tasks: %v", err) + log.Warn("find abandoned jobs: %v", err) return err } - now := timeutil.TimeStampNow() - - updatedJobs := []*actions_model.ActionRunJob{} - - for _, job := range jobs { - job.Status = actions_model.StatusCancelled - job.Stopped = now - updated := false - if err := db.WithTx(ctx, func(ctx context.Context) error { - n, err := actions_model.UpdateRunJob(ctx, job, nil, "status", "stopped") - if err != nil { - return err - } - if err := job.LoadAttributes(ctx); err != nil { - return err - } - updated = n > 0 - return nil - }); err != nil { - log.Warn("cancel abandoned job %v: %v", job.ID, err) - // go on - } - if job.Run == nil || job.Run.Repo == nil { - continue // error occurs during loading attributes, the following code that depends on "Run.Repo" will fail, so ignore and skip - } - if updated { - CreateCommitStatusForRunJobs(ctx, job.Run, job) - updatedJobs = append(updatedJobs, job) - } + updatedJobs, err := actions_model.CancelJobs(ctx, abandonedJobs) + if err != nil { + log.Warn("cancel abandoned jobs: %v", err) } NotifyWorkflowJobsAndRunsStatusUpdate(ctx, updatedJobs) diff --git a/services/actions/clear_tasks_test.go b/services/actions/clear_tasks_test.go new file mode 100644 index 0000000000..7410468f38 --- /dev/null +++ b/services/actions/clear_tasks_test.go @@ -0,0 +1,90 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package actions + +import ( + "testing" + + actions_model "code.gitea.io/gitea/models/actions" + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/models/unittest" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func createConflictingCancellingJob(t *testing.T, concurrencyGroup string, runIndex int64) *actions_model.ActionRunJob { + t.Helper() + + run := &actions_model.ActionRun{ + RepoID: 1, + OwnerID: 2, + TriggerUserID: 2, + WorkflowID: "test.yml", + Index: runIndex, + Ref: "refs/heads/main", + Status: actions_model.StatusBlocked, + } + require.NoError(t, db.Insert(t.Context(), run)) + + attempt := &actions_model.ActionRunAttempt{ + RepoID: run.RepoID, + RunID: run.ID, + Attempt: 1, + TriggerUserID: run.TriggerUserID, + Status: actions_model.StatusBlocked, + ConcurrencyGroup: concurrencyGroup, + } + require.NoError(t, db.Insert(t.Context(), attempt)) + + job := &actions_model.ActionRunJob{ + RunID: run.ID, + RunAttemptID: attempt.ID, + AttemptJobID: 1, + RepoID: run.RepoID, + OwnerID: run.OwnerID, + CommitSHA: "c2d72f548424103f01ee1dc02889c1e2bff816b0", + Name: "conflicting-cancelling-job", + JobID: "conflicting-cancelling-job", + Status: actions_model.StatusCancelling, + ConcurrencyGroup: concurrencyGroup, + } + require.NoError(t, db.Insert(t.Context(), job)) + + return job +} + +func TestShouldBlockJobByConcurrency_CancellingJobBlocks(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + const concurrencyGroup = "test-cancelling-job-blocks" + createConflictingCancellingJob(t, concurrencyGroup, 9903) + + job := &actions_model.ActionRunJob{ + RepoID: 1, + RawConcurrency: concurrencyGroup, + IsConcurrencyEvaluated: true, + ConcurrencyGroup: concurrencyGroup, + } + + shouldBlock, err := shouldBlockJobByConcurrency(t.Context(), job) + require.NoError(t, err) + assert.True(t, shouldBlock) +} + +func TestShouldBlockRunByConcurrency_CancellingJobBlocks(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + const concurrencyGroup = "test-cancelling-run-blocks" + createConflictingCancellingJob(t, concurrencyGroup, 9904) + + attempt := &actions_model.ActionRunAttempt{ + RepoID: 1, + ConcurrencyGroup: concurrencyGroup, + } + + shouldBlock, err := shouldBlockRunByConcurrency(t.Context(), attempt) + require.NoError(t, err) + assert.True(t, shouldBlock) +} diff --git a/services/actions/commit_status.go b/services/actions/commit_status.go index 76c11da7cb..77056eed94 100644 --- a/services/actions/commit_status.go +++ b/services/actions/commit_status.go @@ -181,11 +181,13 @@ func toCommitStatusDescription(job *actions_model.ActionRunJob) string { case actions_model.StatusFailure: return fmt.Sprintf("Failing after %s", job.Duration()) case actions_model.StatusCancelled: - return fmt.Sprintf("Cancelled after %s", job.Duration()) + return fmt.Sprintf("Canceled after %s", job.Duration()) case actions_model.StatusSkipped: return "Skipped" case actions_model.StatusRunning: return "In progress" + case actions_model.StatusCancelling: + return "Canceling" case actions_model.StatusWaiting: return "Waiting to run" case actions_model.StatusBlocked: @@ -201,7 +203,7 @@ func toCommitStatus(status actions_model.Status) commitstatus.CommitStatusState return commitstatus.CommitStatusSuccess case actions_model.StatusFailure, actions_model.StatusCancelled: return commitstatus.CommitStatusFailure - case actions_model.StatusWaiting, actions_model.StatusBlocked, actions_model.StatusRunning: + case actions_model.StatusWaiting, actions_model.StatusBlocked, actions_model.StatusRunning, actions_model.StatusCancelling: return commitstatus.CommitStatusPending case actions_model.StatusSkipped: return commitstatus.CommitStatusSkipped diff --git a/services/actions/commit_status_test.go b/services/actions/commit_status_test.go index fa95a46383..d810d9efc3 100644 --- a/services/actions/commit_status_test.go +++ b/services/actions/commit_status_test.go @@ -28,7 +28,8 @@ func TestCommitStatusDescription(t *testing.T) { }{ {actions_model.StatusSuccess, 100, 102, "Successful in 2s"}, {actions_model.StatusFailure, 100, 130, "Failing after 30s"}, - {actions_model.StatusCancelled, 100, 145, "Cancelled after 45s"}, + {actions_model.StatusCancelled, 100, 145, "Canceled after 45s"}, + {actions_model.StatusCancelling, 0, 0, "Canceling"}, {actions_model.StatusSkipped, 0, 0, "Skipped"}, {actions_model.StatusRunning, 0, 0, "In progress"}, {actions_model.StatusWaiting, 0, 0, "Waiting to run"}, diff --git a/services/actions/notify.go b/services/actions/notify.go index 21ca8c7a14..73b84f4d5c 100644 --- a/services/actions/notify.go +++ b/services/actions/notify.go @@ -18,8 +18,9 @@ func NotifyWorkflowJobsAndRunsStatusUpdate(ctx context.Context, jobs []*actions_ return } - // The input jobs may belong to different runs, so track each affected run. - runs := make(map[int64]*actions_model.ActionRun, len(jobs)) + // The input jobs may belong to different runs, so track each affected run ID + // and reload it later to avoid notifying with stale aggregate status. + runRepoIDs := make(map[int64]int64, len(jobs)) jobsByRunID := make(map[int64][]*actions_model.ActionRunJob) for _, job := range jobs { @@ -29,17 +30,15 @@ func NotifyWorkflowJobsAndRunsStatusUpdate(ctx context.Context, jobs []*actions_ } CreateCommitStatusForRunJobs(ctx, job.Run, job) - if _, ok := runs[job.RunID]; !ok { - runs[job.RunID] = job.Run - } + runRepoIDs[job.RunID] = job.RepoID if _, ok := jobsByRunID[job.RunID]; !ok { jobsByRunID[job.RunID] = make([]*actions_model.ActionRunJob, 0) } jobsByRunID[job.RunID] = append(jobsByRunID[job.RunID], job) } - for _, run := range runs { - NotifyWorkflowRunStatusUpdate(ctx, run) + for runID, repoID := range runRepoIDs { + NotifyWorkflowRunStatusUpdateWithReload(ctx, repoID, runID) } for _, jobs := range jobsByRunID { diff --git a/services/actions/task.go b/services/actions/task.go index 9dc3c9a34b..b44205879b 100644 --- a/services/actions/task.go +++ b/services/actions/task.go @@ -35,7 +35,7 @@ func PickTask(ctx context.Context, runner *actions_model.ActionRunner) (*runnerv return nil, false, err } if has { - if task.Status == actions_model.StatusWaiting || task.Status == actions_model.StatusRunning || task.Status == actions_model.StatusBlocked { + if task.Status.In(actions_model.StatusWaiting, actions_model.StatusRunning, actions_model.StatusBlocked, actions_model.StatusCancelling) { return nil, false, nil } // task has been finished, remove it diff --git a/templates/repo/actions/view_component.tmpl b/templates/repo/actions/view_component.tmpl index 67926276c0..827c04d185 100644 --- a/templates/repo/actions/view_component.tmpl +++ b/templates/repo/actions/view_component.tmpl @@ -22,6 +22,7 @@ data-locale-status-unknown="{{ctx.Locale.Tr "actions.status.unknown"}}" data-locale-status-waiting="{{ctx.Locale.Tr "actions.status.waiting"}}" data-locale-status-running="{{ctx.Locale.Tr "actions.status.running"}}" + data-locale-status-cancelling="{{ctx.Locale.Tr "actions.status.cancelling"}}" data-locale-status-success="{{ctx.Locale.Tr "actions.status.success"}}" data-locale-status-failure="{{ctx.Locale.Tr "actions.status.failure"}}" data-locale-status-cancelled="{{ctx.Locale.Tr "actions.status.cancelled"}}" diff --git a/templates/repo/icons/action_status.tmpl b/templates/repo/icons/action_status.tmpl index 4f381bdb0e..1711c61b2b 100644 --- a/templates/repo/icons/action_status.tmpl +++ b/templates/repo/icons/action_status.tmpl @@ -1,7 +1,7 @@ {{/* Status icons used for runs, jobs and steps. Template Attributes: -* Status: one of success, skipped, waiting, blocked, running, failure, cancelled, unknown +* Status: one of success, skipped, waiting, blocked, running, failure, cancelled, cancelling, unknown * Size: icon size in pixels (default 16) * ClassName: additional CSS classes * IconVariant: "circle-fill" → octicon-check-circle-fill / octicon-x-circle-fill @@ -23,6 +23,8 @@ Keep this template in sync with web_src/js/components/ActionStatusIcon.vue. {{svg "octicon-blocked" $size (printf "tw-text-yellow %s" $className)}} {{else if eq .Status "running"}} {{svg "gitea-running" $size (printf "tw-text-yellow rotate-clockwise %s" $className)}} +{{else if eq .Status "cancelling"}} + {{svg "octicon-stop" $size (printf "tw-text-yellow %s" $className)}} {{else}}{{/*failure, unknown*/}} {{svg (Iif $circleFill "octicon-x-circle-fill" "octicon-x") $size (printf "tw-text-red %s" $className)}} {{end}} diff --git a/tests/integration/actions_concurrency_test.go b/tests/integration/actions_concurrency_test.go index 6460af0fd2..ce90256ed2 100644 --- a/tests/integration/actions_concurrency_test.go +++ b/tests/integration/actions_concurrency_test.go @@ -1560,6 +1560,9 @@ jobs: // run2 is blocked because it is blocked by workflow1's concurrency group "test-group" assert.Equal(t, actions_model.StatusBlocked, run2.Status) + // complete wf1-job1 + runner.execTask(t, w1j1Task, &mockTaskOutcome{result: runnerv1.Result_RESULT_SUCCESS}) + // mock time fakeNow := now.Add(setting.Actions.AbandonedJobTimeout) timeutil.MockSet(fakeNow) diff --git a/web_src/js/components/ActionStatusIcon.vue b/web_src/js/components/ActionStatusIcon.vue index 85ef61c604..12da669ac4 100644 --- a/web_src/js/components/ActionStatusIcon.vue +++ b/web_src/js/components/ActionStatusIcon.vue @@ -1,11 +1,11 @@