feat: display detailed action run diagnostics #9966

aahlenst · 2025-11-04T21:48:22+01:00

aahlenst commented

2025-11-04 21:48:22 +01:00

Forgejo Actions allows variables in jobs.<job_id>.runs-on. However, the action list skips checking whether a suitable runner is available if an expression contains variables. That hampers a user's ability to figure out whether an expression was evaluated correctly and why a job might not be picked up by an available runner.

This PR adds the ability to surface more complex and additional diagnostic information on the action view screen. Previously, only a job's status (waiting, running, ...) was displayed. Now, extended messages like "Waiting for a runner with the following labels: docker, trixie" are displayed with the possibility to show multiple messages simultaneously.

How it looked before:

How it looks after updating Forgejo without reloading the window:

How it looks afterwards with a single label:

How it looks afterwards with multiple labels:

Checklist

The contributor guide contains information that will be helpful to first time contributors. There also are a few conditions for merging Pull Requests in Forgejo repositories. You are also welcome to join the Forgejo development chatroom.

Tests

I added test coverage for Go changes...
- in their respective *_test.go for unit tests.
- 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).

Documentation

I created a pull request to the documentation to explain to Forgejo users how to use this change.
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.

Release notes

Features
- PR: display detailed action run diagnostics

Forgejo Actions allows variables in `jobs.<job_id>.runs-on`. However, the action list [skips checking whether a suitable runner](https://codeberg.org/forgejo/forgejo/src/commit/c3412d0579acfc702224f1785fc44e5695daee90/routers/web/repo/actions/actions.go#L114-L148) is available if an expression contains variables. That hampers a user's ability to figure out whether an expression was evaluated correctly and why a job might not be picked up by an available runner. This PR adds the ability to surface more complex and additional diagnostic information on the action view screen. Previously, only a job's status (waiting, running, ...) was displayed. Now, extended messages like "Waiting for a runner with the following labels: docker, trixie" are displayed with the possibility to show multiple messages simultaneously. How it looked before: ![old](/attachments/019e4e83-d44e-4143-8df0-7fceb611a3bd) How it looks after updating Forgejo without reloading the window: ![old-after-update](/attachments/e909af81-cf9e-4f44-a011-75585a2d1950) How it looks afterwards with a single label: ![new-single-label](/attachments/72f4a862-e23d-4ab5-9f96-09545954f982) How it looks afterwards with multiple labels: ![new-multiple-labels](/attachments/53036d7b-3589-4eeb-bad1-4da4cd5ff4b5) ## 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... - [x] 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.  ## Release notes  - Features - [PR](https://codeberg.org/forgejo/forgejo/pulls/9966): display detailed action run diagnostics

old.png

93 KiB

old-after-update.png

92 KiB

new-single-label.png

101 KiB

new-multiple-labels.png

103 KiB

aahlenst added the

test

needed

label

2025-11-04 21:48:22 +01:00

aahlenst requested reviews from Beowulf, Gusted, 0ko

2025-11-04 21:48:29 +01:00

earl-warren added

and removed

labels

2025-11-05 11:23:58 +01:00

Gusted added the

enhancement/feature

label

2025-11-05 15:31:32 +01:00

mfenniak reviewed

2025-11-10 20:56:44 +01:00

mfenniak left a comment

I think this looks great. Please review the attached comments and update the PR description to include screenshots of the new feature and the standard PR template.

models/actions/run_job.go Outdated

					
				@ -232,0 +240,4 @@

					switch job.Status {

					case StatusWaiting:

						diagnostics = append(diagnostics, lang.TrString("actions.status.diagnostics.waiting", strings.Join(job.RunsOn, ", ")))

Should use TrPluralString to allow proper pluralization of the translation string based upon how many entries are in job.RunsOn.

Should use `TrPluralString` to allow proper pluralization of the translation string based upon how many entries are in `job.RunsOn`.

mfenniak marked this conversation as resolved

models/actions/run_job.go Outdated

					
				@ -232,0 +245,4 @@

						diagnostics = append(diagnostics, job.Status.LocaleString(lang))

					}

					if job.Run.NeedApproval {

With the merge of #9397, I've performed a static review and it seems that checking NeedApproval will still work correctly for this check -- but a rebase and verification is worthwhile.

With the merge of #9397, I've performed a static review and it seems that checking `NeedApproval` will still work correctly for this check -- but a rebase and verification is worthwhile.

There's a unit test that covers that the message is generated if job.Run.NeedApproval is set.

There's a unit test that covers that the message is generated if `job.Run.NeedApproval` is set.

mfenniak marked this conversation as resolved

options/locale/locale_en-US.ini Outdated

					
				@ -3746,6 +3746,8 @@ status.cancelled = Canceled

				status.skipped = Skipped

				status.blocked = Blocked

				status.diagnostics.waiting = Waiting for a runner with the following labels: %s

New localization strings should be placed in locale_en-US.json
With the use of TrPluralString (noted in another comment) we could fix the pluralization here so that it isn't always "labels" when it might be singular ("label").

- New localization strings should be placed in `locale_en-US.json` - With the use of `TrPluralString` (noted in another comment) we could fix the pluralization here so that it isn't always "labels" when it might be singular ("label").

mfenniak marked this conversation as resolved

routers/web/repo/actions/view.go Outdated

					
				@ -184,3 +184,3 @@

				type ViewCurrentJob struct {

					Title       string         `json:"title"`

					Detail      string         `json:"detail"`

					Details     []string       `json:"details"`

I'm thinking about what will happen when someone has the Forgejo UI open viewing an action run, and Forgejo is upgraded to include this new feature. The next API call to poll will get the value details and the value detail will be absent. The older version of the JS will continue to run until the page is reloaded, and I believe it will likely work with no errors -- although it won't have the detail field anymore, it will just change the rendering of {{ currentJob.detail }} to possibly display undefined.

If you agree with that assessment that there's no major problem caused during an upgrade, then we can resolve this comment.

I'm thinking about what will happen when someone has the Forgejo UI open viewing an action run, and Forgejo is upgraded to include this new feature. The next API call to poll will get the value `details` and the value `detail` will be absent. The older version of the JS will continue to run until the page is reloaded, and I believe it will likely work with no errors -- although it won't have the `detail` field anymore, it will just change the rendering of `{{ currentJob.detail }}` to possibly display undefined. If you agree with that assessment that there's no major problem caused during an upgrade, then we can resolve this comment.

There's now a screenshot above that shows what's going to happen.

mfenniak marked this conversation as resolved

aahlenst force-pushed action-run-diagnostics from 913f9e6cbd

testing / frontend-checks (pull_request) Successful in 2m2s

Details

testing / backend-checks (pull_request) Successful in 3m31s

Details

testing / test-e2e (pull_request) Failing after 6m41s

Details

testing / test-unit (pull_request) Successful in 7m49s

Details

testing / test-remote-cacher (redis) (pull_request) Successful in 1m50s

Details

testing / test-remote-cacher (valkey) (pull_request) Successful in 1m52s

Details

testing / test-remote-cacher (garnet) (pull_request) Successful in 1m40s

Details

testing / test-remote-cacher (redict) (pull_request) Successful in 1m40s

Details

testing / test-mysql (pull_request) Successful in 24m14s

Details

testing / test-sqlite (pull_request) Successful in 29m44s

Details

testing / test-pgsql (pull_request) Successful in 35m12s

Details

testing / security-check (pull_request) Successful in 1m3s

Details

issue-labels / backporting (pull_request_target) Has been skipped

Details

issue-labels / cascade (pull_request_target) Has been skipped

Details

requirements / merge-conditions (pull_request) Successful in 4s

Details

issue-labels / release-notes (pull_request_target) Has been skipped

Details

to 09fe488363

issue-labels / backporting (pull_request_target) Has been skipped

Details

issue-labels / cascade (pull_request_target) Has been skipped

Details

requirements / merge-conditions (pull_request) Successful in 1s

Details

issue-labels / release-notes (pull_request_target) Successful in 31s

Details

2025-11-10 22:32:42 +01:00

Compare

aahlenst reviewed

2025-11-10 22:37:49 +01:00

options/locale_next/locale_en-US.json

					
				@ -181,2 +181,4 @@

				        "other": "%s downloads"

				    },

				    "actions.status.diagnostics.waiting": {

				        "one": "Waiting for a runner with the following label: %s",

It seems I cannot specify a message for the "zero" case. That is only accepted in some other languages.

mfenniak marked this conversation as resolved

aahlenst commented

2025-11-10 22:38:56 +01:00

It's now using TrPluralString and I also made a rebase. I've added a couple of screenshots.

It's now using `TrPluralString` and I also made a rebase. I've added a couple of screenshots.

mfenniak approved these changes

2025-11-11 02:35:15 +01:00

mfenniak added the

worth a release-note

label

2025-11-11 02:35:27 +01:00

forgejo-release-notes-assistant commented

2025-11-11 02:36:19 +01:00

Where does that come from?

The following is a preview of the release notes for this pull request, as they will appear in the upcoming release. They are derived from the content of the `release-notes/9966.md` file, if it exists, or the title of the pull request. They were also added at the bottom of the description of this pull request for easier reference.

This message and the release notes originate from a call to the release-notes-assistant.

@@ -42,2 +42,10 @@
 - [ ] 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.
+
+<!--start release-notes-assistant-->
+
+## Release notes
+<!--URL:https://codeberg.org/forgejo/forgejo-->
+- Features
+  - [PR](https://codeberg.org/forgejo/forgejo/pulls/9966): <!--number 9966 --><!--line 0 --><!--description ZGlzcGxheSBkZXRhaWxlZCBhY3Rpb24gcnVuIGRpYWdub3N0aWNz-->display detailed action run diagnostics<!--description-->
+<!--end release-notes-assistant-->

Release notes

Features
- PR: display detailed action run diagnostics

<details> <summary>Where does that come from?</summary> The following is a preview of the release notes for this pull request, as they will appear in the upcoming release. They are derived from the content of the `release-notes/9966.md` file, if it exists, or the title of the pull request. They were also added at the bottom of the description of this pull request for easier reference. This message and the release notes originate from a call to the [release-notes-assistant](https://code.forgejo.org/forgejo/release-notes-assistant). ```diff @@ -42,2 +42,10 @@ - [ ] 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. + + + +## Release notes + +- Features + - [PR](https://codeberg.org/forgejo/forgejo/pulls/9966): display detailed action run diagnostics + ``` </details>  ## Release notes  - Features - [PR](https://codeberg.org/forgejo/forgejo/pulls/9966): display detailed action run diagnostics

mfenniak commented

2025-11-11 02:40:09 +01:00

For some reason CI hasn't started on the last force push; I'm going to do an "Update branch by merge" to generate a new commit.

mfenniak added 1 commit

2025-11-11 02:40:20 +01:00

Merge branch 'forgejo' into action-run-diagnostics

requirements / merge-conditions (pull_request) Successful in 2s

Details

issue-labels / release-notes (pull_request_target) Successful in 44s

Details

testing / frontend-checks (pull_request) Successful in 1m44s

Details