From c7d23fa6e84417e359d501cc3bca2c1dfd6eed95 Mon Sep 17 00:00:00 2001 From: Mathieu Fenniak Date: Thu, 5 Feb 2026 22:29:51 +0100 Subject: [PATCH] fix: when expanding a dynamic matrix, original 'needs' access was lost (#11164) Fixes #11163. When expanding a dynamic matrix (or any other dynamic job), the references to the original `needs` of the jobs are lost. This is manually tested, and moderately covered by an automated test. Will follow-up with an end-to-end test after a regression run is complete. ## 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. - [ ] 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 - [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. *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/11164 Reviewed-by: Andreas Ahlenstorf Co-authored-by: Mathieu Fenniak Co-committed-by: Mathieu Fenniak --- services/actions/job_emitter.go | 16 +++++++++++++++- services/actions/job_emitter_test.go | 17 ++++++++++++----- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/services/actions/job_emitter.go b/services/actions/job_emitter.go index fe3cc1b38a..bf4881ea04 100644 --- a/services/actions/job_emitter.go +++ b/services/actions/job_emitter.go @@ -324,7 +324,7 @@ func tryHandleIncompleteMatrix(ctx context.Context, blockedJob *actions_model.Ac // it to be a "persistent incomplete" job with some error that needs to be reported to the user. If the // re-evaluated job has a different job ID, then it's likely an expanded job -- such as from a reusable workflow // -- which could have it's own `needs` that allows it to expand into a correct job in the future. - jobID, _ := swf.Job() + jobID, job := swf.Job() if jobID == blockedJob.JobID { if swf.IncompleteMatrix { errorCode, errorDetails := persistentIncompleteMatrixError(blockedJob, swf.IncompleteMatrixNeeds) @@ -349,6 +349,20 @@ func tryHandleIncompleteMatrix(ctx context.Context, blockedJob *actions_model.Ac return behaviourIgnoreAllJobsInRun, nil } } + + // Original job had a `needs: ...blockedJob.Needs...`. Even though we've now expanded that job, which would + // evaluate any ${{ needs.... }} reference that is required for expansion, this job could still have other + // reasons to require acccess to those needs variables. We need to reinsert those `needs` into the new job so + // that those job's outputs and results are made available to this new job. + newNeeds := append(job.Needs(), blockedJob.Needs...) + err := job.RawNeeds.Encode(newNeeds) + if err != nil { + return behaviourError, fmt.Errorf("failure to encode newNeeds: %w", err) + } + err = swf.SetJob(jobID, job) + if err != nil { + return behaviourError, fmt.Errorf("failure to reencode updated job: %w", err) + } } err = db.WithTx(ctx, func(ctx context.Context) error { diff --git a/services/actions/job_emitter_test.go b/services/actions/job_emitter_test.go index caccf75434..6899508f37 100644 --- a/services/actions/job_emitter_test.go +++ b/services/actions/job_emitter_test.go @@ -326,6 +326,12 @@ func Test_tryHandleIncompleteMatrix(t *testing.T) { runJobID: 601, consumed: true, runJobNames: []string{"define-matrix", "produce-artifacts (blue)", "produce-artifacts (green)", "produce-artifacts (red)"}, + needs: map[string][]string{ + "define-matrix": nil, + "produce-artifacts (blue)": {"define-matrix"}, + "produce-artifacts (green)": {"define-matrix"}, + "produce-artifacts (red)": {"define-matrix"}, + }, }, { name: "needs an incomplete job", @@ -490,8 +496,8 @@ func Test_tryHandleIncompleteMatrix(t *testing.T) { }, needs: map[string][]string{ "define-workflow-call": nil, - "inner my-workflow-input": nil, - "perform-workflow-call": {"perform-workflow-call.inner_job"}, + "inner my-workflow-input": {"define-workflow-call"}, + "perform-workflow-call": {"define-workflow-call", "perform-workflow-call.inner_job"}, }, }, // Before reusable workflow expansion, there weren't any cases where evaluating a job in the job emitter could @@ -515,9 +521,10 @@ func Test_tryHandleIncompleteMatrix(t *testing.T) { }, needs: map[string][]string{ "define-workflow-call": nil, - "inner define-runs-on my-workflow-input": nil, - "inner incomplete-job my-workflow-input": {"perform-workflow-call.define-runs-on"}, + "inner define-runs-on my-workflow-input": {"define-workflow-call"}, + "inner incomplete-job my-workflow-input": {"define-workflow-call", "perform-workflow-call.define-runs-on"}, "perform-workflow-call": { + "define-workflow-call", "perform-workflow-call.define-runs-on", "perform-workflow-call.scalar-job", }, @@ -661,7 +668,7 @@ func Test_tryHandleIncompleteMatrix(t *testing.T) { if tt.needs != nil { for _, j := range allJobsInRun { expected, ok := tt.needs[j.Name] - if assert.Truef(t, ok, "unable to find runsOn[%q] in test case", j.Name) { + if assert.Truef(t, ok, "unable to find needs[%q] in test case", j.Name) { slices.Sort(j.Needs) slices.Sort(expected) assert.Equalf(t, expected, j.Needs, "comparing needs expectations for job %q", j.Name)