From 2ed98ac848a3b25d0be5d365535c64a14a7a6476 Mon Sep 17 00:00:00 2001 From: Mathieu Fenniak Date: Wed, 22 Apr 2026 13:41:25 +0200 Subject: [PATCH] fix: resolve outer workflow call to success, not failure, on inner job skip (#12224) If one or more of a workflow expansion's inner jobs are status "skipped", consider that as a success, rather than a failure. Fixes https://code.forgejo.org/forgejo/runner/issues/1490. ## 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 - I added test coverage for Go changes... - [x] in their respective `*_test.go` for unit tests. - [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server. - I ran... - [x] `make pr-go` before pushing ### 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 - [x] 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. - [ ] 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. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12224 Reviewed-by: Michael Kriese Reviewed-by: Andreas Ahlenstorf --- services/actions/job_emitter.go | 7 +++-- services/actions/job_emitter_test.go | 42 ++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/services/actions/job_emitter.go b/services/actions/job_emitter.go index 593e17517a..0a9c297033 100644 --- a/services/actions/job_emitter.go +++ b/services/actions/job_emitter.go @@ -192,7 +192,7 @@ func (r *jobStatusResolver) resolve() map[int64]actions_model.Status { if status != actions_model.StatusBlocked { continue } - allDone, allSucceed := true, true + allDone, allSucceed, allSucceedOrSkip := true, true, true for _, need := range r.needs[id] { needStatus := r.statuses[need] if !needStatus.IsDone() { @@ -201,13 +201,16 @@ func (r *jobStatusResolver) resolve() map[int64]actions_model.Status { if needStatus.In(actions_model.StatusFailure, actions_model.StatusCancelled, actions_model.StatusSkipped) { allSucceed = false } + if needStatus.In(actions_model.StatusFailure, actions_model.StatusCancelled) { + allSucceedOrSkip = false + } } if allDone { if isWorkflowCallOuterJob, _ := r.jobMap[id].IsWorkflowCallOuterJob(); isWorkflowCallOuterJob { // If the dependent job was a workflow call outer job, then options aren't waiting/skipped, but rather // success/failure. checkJobsOfRun will do additional work in these cases to "finish" the workflow call // job as well. - if allSucceed { + if allSucceedOrSkip { isIncompleteMatrix, _, _ := r.jobMap[id].HasIncompleteMatrix() isIncompleteWith, _, _, _ := r.jobMap[id].HasIncompleteWith() if isIncompleteMatrix || isIncompleteWith { diff --git a/services/actions/job_emitter_test.go b/services/actions/job_emitter_test.go index 44e9163102..a7e3b7467b 100644 --- a/services/actions/job_emitter_test.go +++ b/services/actions/job_emitter_test.go @@ -146,6 +146,27 @@ jobs: ` name: test on: push +jobs: + job2: + if: false + uses: ./.forgejo/workflows/reusable.yml +__metadata: + workflow_call_id: b5a9f46f1f2513d7777fde50b169d323a6519e349cc175484c947ac315a209ed +`)}, + }, + want: map[int64]actions_model.Status{ + 3: actions_model.StatusSuccess, + }, + }, + { + name: "unblocked workflow call outer job with success and skip", + jobs: actions_model.ActionJobList{ + {ID: 1, JobID: "job1.innerjob1", Status: actions_model.StatusSuccess, Needs: []string{}}, + {ID: 2, JobID: "job1.innerjob2", Status: actions_model.StatusSkipped, Needs: []string{}}, + {ID: 3, JobID: "job1", Status: actions_model.StatusBlocked, Needs: []string{"job1.innerjob1", "job1.innerjob2"}, WorkflowPayload: []byte( + ` +name: test +on: push jobs: job2: if: false @@ -219,6 +240,27 @@ __metadata: ` name: test on: push +jobs: + job2: + if: false + uses: ./.forgejo/workflows/reusable.yml +__metadata: + workflow_call_id: b5a9f46f1f2513d7777fde50b169d323a6519e349cc175484c947ac315a209ed +`)}, + }, + want: map[int64]actions_model.Status{ + 3: actions_model.StatusFailure, + }, + }, + { + name: "unblocked workflow call outer job with internal failure", + jobs: actions_model.ActionJobList{ + {ID: 1, JobID: "job1.innerjob1", Status: actions_model.StatusSkipped, Needs: []string{}}, + {ID: 2, JobID: "job1.innerjob2", Status: actions_model.StatusFailure, Needs: []string{}}, + {ID: 3, JobID: "job1", Status: actions_model.StatusBlocked, Needs: []string{"job1.innerjob1", "job1.innerjob2"}, WorkflowPayload: []byte( + ` +name: test +on: push jobs: job2: if: false