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/<pull request number>.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 <aahlenst@noreply.codeberg.org>
Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net>
Co-committed-by: Mathieu Fenniak <mathieu@fenniak.net>

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 {

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)