Actions Done Notification (#7491)

This PR depends on https://codeberg.org/forgejo/forgejo/pulls/7510

This PR renames UpdateRunJob to UpdateRunJobWithoutNotification and UpdateRun to  UpdateRunWithoutNotification and implements wrapper functions that also call the new ActionRunNowDone notification when needed.

This PR can be reviewed commit-by-commit.

# Things to Test
- [x] GetRunBefore
- [ ] integration test for sendActionRunNowDoneNotificationIfNeeded, UpdateRun and UpdateRunJob

## Checklist

The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org).

### Tests

- I added test coverage for Go changes...
  - [x] in their respective `*_test.go` for unit tests.
  - [x] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- I added test coverage for JavaScript changes...
  - [ ] in `web_src/js/*.test.js` if it can be unit tested.
  - [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)).

### Documentation

- [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- [x] I did not document these changes and I do not expect someone else to do it.

### Release notes

- [ ] I do not want this change to show in the release notes.
- [ ] I want the title to show in the release notes with a link to this pull request.
- [ ] I want the content of the `release-notes/<pull request number>.md` to be be used for the release notes instead of the title.

Co-authored-by: nobody <nobody@example.com>
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/7491
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Co-authored-by: christopher-besch <mail@chris-besch.com>
Co-committed-by: christopher-besch <mail@chris-besch.com>

models/actions/main_test.go

@@ -13,6 +13,7 @@ func TestMain(m *testing.M) {
 	unittest.MainTest(m, &unittest.TestOptions{
 		FixtureFiles: []string{
 			"action_runner.yml",
+			"repository.yml",
 			"action_runner_token.yml",
 		},
 	})

models/actions/run.go

@@ -187,6 +187,7 @@ func updateRepoRunsNumbers(ctx context.Context, repo *repo_model.Repository) err
 
 // InsertRun inserts a run
 // The title will be cut off at 255 characters if it's longer than 255 characters.
+// We don't have to send the ActionRunNowDone notification here because there are no runs that start in a not done status.
 func InsertRun(ctx context.Context, run *ActionRun, jobs []*jobparser.SingleWorkflow) error {
 	ctx, commiter, err := db.TxContext(ctx)
 	if err != nil {
@@ -272,6 +273,18 @@ func GetLatestRun(ctx context.Context, repoID int64) (*ActionRun, error) {
 	return &run, nil
 }
 
+// GetRunBefore returns the last run that completed a given timestamp (not inclusive).
+func GetRunBefore(ctx context.Context, repoID int64, timestamp timeutil.TimeStamp) (*ActionRun, error) {
+	var run ActionRun
+	has, err := db.GetEngine(ctx).Where("repo_id=? AND stopped IS NOT NULL AND stopped<?", repoID, timestamp).OrderBy("stopped DESC").Limit(1).Get(&run)
+	if err != nil {
+		return nil, err
+	} else if !has {
+		return nil, fmt.Errorf("run before: %w", util.ErrNotExist)
+	}
+	return &run, nil
+}
+
 func GetLatestRunForBranchAndWorkflow(ctx context.Context, repoID int64, branch, workflowFile, event string) (*ActionRun, error) {
 	var run ActionRun
 	q := db.GetEngine(ctx).Where("repo_id=?", repoID).And("workflow_id=?", workflowFile)
@@ -320,7 +333,9 @@ func GetRunByIndex(ctx context.Context, repoID, index int64) (*ActionRun, error)
 // UpdateRun updates a run.
 // It requires the inputted run has Version set.
 // It will return error if the version is not matched (it means the run has been changed after loaded).
-func UpdateRun(ctx context.Context, run *ActionRun, cols ...string) error {
+// All calls to UpdateRunWithoutNotification that change run.Status from a not done status to a done status must call the ActionRunNowDone notification channel.
+// Use the wrapper function UpdateRun instead.
+func UpdateRunWithoutNotification(ctx context.Context, run *ActionRun, cols ...string) error {
 	sess := db.GetEngine(ctx).ID(run.ID)
 	if len(cols) > 0 {
 		sess.Cols(cols...)

models/actions/run_job.go

@@ -101,7 +101,9 @@ func GetRunJobsByRunID(ctx context.Context, runID int64) ([]*ActionRunJob, error
 	return jobs, nil
 }
 
-func UpdateRunJob(ctx context.Context, job *ActionRunJob, cond builder.Cond, cols ...string) (int64, error) {
+// All calls to UpdateRunJobWithoutNotification that change run.Status for any run from a not done status to a done status must call the ActionRunNowDone notification channel.
+// Use the wrapper function UpdateRunJob instead.
+func UpdateRunJobWithoutNotification(ctx context.Context, job *ActionRunJob, cond builder.Cond, cols ...string) (int64, error) {
 	e := db.GetEngine(ctx)
 
 	sess := e.ID(job.ID)
@@ -154,7 +156,8 @@ func UpdateRunJob(ctx context.Context, job *ActionRunJob, cond builder.Cond, col
 		if run.Stopped.IsZero() && run.Status.IsDone() {
 			run.Stopped = timeutil.TimeStampNow()
 		}
-		if err := UpdateRun(ctx, run, "status", "started", "stopped"); err != nil {
+		// As the caller has to ensure the ActionRunNowDone notification is sent we can ignore doing so here.
+		if err := UpdateRunWithoutNotification(ctx, run, "status", "started", "stopped"); err != nil {
 			return 0, fmt.Errorf("update run %d: %w", run.ID, err)
 		}
 	}

models/actions/run_test.go

@@ -0,0 +1,96 @@
+// Copyright 2025 The Forgejo Authors. All rights reserved.
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package actions
+
+import (
+	"testing"
+	"time"
+
+	"forgejo.org/models/db"
+	"forgejo.org/models/unittest"
+	"forgejo.org/modules/timeutil"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestGetRunBefore(t *testing.T) {
+	require.NoError(t, unittest.PrepareTestDatabase())
+
+	// this repo is part of the test database requiring loading "repository.yml" in main_test.go
+	var repoID int64 = 1
+
+	workflowID := "test_workflow"
+
+	// third completed run
+	time1, err := time.Parse(time.RFC3339, "2024-07-31T15:47:55+08:00")
+	require.NoError(t, err)
+	timeutil.MockSet(time1)
+	run1 := ActionRun{
+		ID:         1,
+		Index:      1,
+		RepoID:     repoID,
+		Stopped:    timeutil.TimeStampNow(),
+		WorkflowID: workflowID,
+	}
+
+	// fourth completed run
+	time2, err := time.Parse(time.RFC3339, "2024-08-31T15:47:55+08:00")
+	require.NoError(t, err)
+	timeutil.MockSet(time2)
+	run2 := ActionRun{
+		ID:         2,
+		Index:      2,
+		RepoID:     repoID,
+		Stopped:    timeutil.TimeStampNow(),
+		WorkflowID: workflowID,
+	}
+
+	// second completed run
+	time3, err := time.Parse(time.RFC3339, "2024-07-31T15:47:54+08:00")
+	require.NoError(t, err)
+	timeutil.MockSet(time3)
+	run3 := ActionRun{
+		ID:         3,
+		Index:      3,
+		RepoID:     repoID,
+		Stopped:    timeutil.TimeStampNow(),
+		WorkflowID: workflowID,
+	}
+
+	// first completed run
+	time4, err := time.Parse(time.RFC3339, "2024-06-30T15:47:54+08:00")
+	require.NoError(t, err)
+	timeutil.MockSet(time4)
+	run4 := ActionRun{
+		ID:         4,
+		Index:      4,
+		RepoID:     repoID,
+		Stopped:    timeutil.TimeStampNow(),
+		WorkflowID: workflowID,
+	}
+	require.NoError(t, db.Insert(db.DefaultContext, &run1))
+	runBefore, err := GetRunBefore(db.DefaultContext, repoID, run1.Stopped)
+	// there is no run before run1
+	require.Error(t, err)
+	require.Nil(t, runBefore)
+
+	// now there is only run3 before run1
+	require.NoError(t, db.Insert(db.DefaultContext, &run3))
+	runBefore, err = GetRunBefore(db.DefaultContext, repoID, run1.Stopped)
+	require.NoError(t, err)
+	assert.Equal(t, run3.ID, runBefore.ID)
+
+	// there still is only run3 before run1
+	require.NoError(t, db.Insert(db.DefaultContext, &run2))
+	runBefore, err = GetRunBefore(db.DefaultContext, repoID, run1.Stopped)
+	require.NoError(t, err)
+	assert.Equal(t, run3.ID, runBefore.ID)
+
+	// run4 is further away from run1
+	require.NoError(t, db.Insert(db.DefaultContext, &run4))
+	runBefore, err = GetRunBefore(db.DefaultContext, repoID, run1.Stopped)
+	require.NoError(t, err)
+	assert.Equal(t, run3.ID, runBefore.ID)
+}

models/actions/task.go

@@ -311,7 +311,8 @@ func CreateTaskForRunner(ctx context.Context, runner *ActionRunner) (*ActionTask
 	}
 
 	job.TaskID = task.ID
-	if n, err := UpdateRunJob(ctx, job, builder.Eq{"task_id": 0}); err != nil {
+	// We never have to send a notification here because the job is started with a not done status.
+	if n, err := UpdateRunJobWithoutNotification(ctx, job, builder.Eq{"task_id": 0}); err != nil {
 		return nil, false, err
 	} else if n != 1 {
 		return nil, false, nil