From ce27a5993cc71f3d0877e54b66584df85b0943f9 Mon Sep 17 00:00:00 2001 From: Andreas Ahlenstorf Date: Fri, 3 Apr 2026 16:05:09 +0200 Subject: [PATCH] fix: superfluous increment of ActionTask attempt breaks job view (#11956) https://codeberg.org/forgejo/forgejo/pulls/11750 missed a place where the attempt number is incremented independently. This caused the job view to break when running a reusable workflow with workflow expansion. ## Checklist The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. All work and communication must conform to Forgejo's [AI Agreement](https://codeberg.org/forgejo/governance/src/branch/main/AIAgreement.md). 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 for Go changes (can be removed for JavaScript changes) - I added test coverage for Go changes... - [ ] in their respective `*_test.go` for unit tests. - [x] in the `tests/integration` directory if it involves interactions with a live Forgejo server. - I ran... - [x] `make pr-go` before pushing ### Tests for JavaScript changes (can be removed for Go changes) - 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 - [ ] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change. - [x] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change. *The decision if the pull request will be shown in the release notes is up to the mergers / release team.* The content of the `release-notes/.md` file will serve as the basis for the release notes. If the file does not exist, the title of the pull request will be used instead. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11956 Reviewed-by: Mathieu Fenniak Co-authored-by: Andreas Ahlenstorf Co-committed-by: Andreas Ahlenstorf --- models/actions/task.go | 4 +--- .../Test_tryHandleWorkflowCallOuterJob/action_run_job.yml | 6 +++++- services/actions/job_emitter.go | 1 - 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/models/actions/task.go b/models/actions/task.go index 1a208dad9d..ed2cab60e8 100644 --- a/models/actions/task.go +++ b/models/actions/task.go @@ -451,9 +451,7 @@ func CreateTaskForRunner(ctx context.Context, runner *ActionRunner, requestKey, } // Placeholder tasks are created when the status/content of an [ActionRunJob] is resolved by Forgejo without dispatch to -// a runner, specifically in the case of a workflow call's outer job. It is the responsibility of the caller to -// increment the job's Attempt field before invoking this method, and to update that field in the database, so that -// reruns can function for placeholder tasks and provide updated outputs. +// a runner, specifically in the case of a workflow call's outer job. func CreatePlaceholderTask(ctx context.Context, job *ActionRunJob, outputs map[string]string) (*ActionTask, error) { actionTask := &ActionTask{ JobID: job.ID, diff --git a/services/actions/Test_tryHandleWorkflowCallOuterJob/action_run_job.yml b/services/actions/Test_tryHandleWorkflowCallOuterJob/action_run_job.yml index 03449040ca..f710f4bf2e 100644 --- a/services/actions/Test_tryHandleWorkflowCallOuterJob/action_run_job.yml +++ b/services/actions/Test_tryHandleWorkflowCallOuterJob/action_run_job.yml @@ -1,6 +1,7 @@ # Case 600 -- workflow that is not a workflow call outer job - id: 600 + attempt: 1 status: 1 # success started: 1683636528 workflow_payload: | @@ -18,6 +19,7 @@ # contexts should be considered as those in `on.workflow_call.outputs`. - id: 601 + attempt: 1 run_id: 900 status: 1 # success started: 1683636528 @@ -44,6 +46,7 @@ workflow_call_id: b5a9f46f1f2513d7777fde50b169d323a6519e349cc175484c947ac315a209ed - # inner job of run 601 id: 602 + attempt: 1 run_id: 900 status: 1 # success job_id: outer-job.inner-job @@ -54,7 +57,7 @@ - id: 603 run_id: 901 - attempt: 1 + attempt: 2 status: 1 # success started: 1683636528 needs: ["outer-job.inner-job"] @@ -81,6 +84,7 @@ - # inner job of run 603 id: 604 run_id: 901 + attempt: 2 status: 1 # success job_id: outer-job.inner-job task_id: 101 diff --git a/services/actions/job_emitter.go b/services/actions/job_emitter.go index 02ad9d66f5..b23918e42f 100644 --- a/services/actions/job_emitter.go +++ b/services/actions/job_emitter.go @@ -597,7 +597,6 @@ func tryHandleWorkflowCallOuterJob(ctx context.Context, job *actions_model.Actio ) // Insert a placeholder task with all the computed outputs - job.Attempt++ actionTask, err := actions_model.CreatePlaceholderTask(ctx, job, outputs) if err != nil { return nil, fmt.Errorf("failure to insert placeholder task: %w", err)