From 6e5dbfa1692e6a5ca48470a79c1ddceca1143bc9 Mon Sep 17 00:00:00 2001 From: Mathieu Fenniak Date: Tue, 12 May 2026 22:41:07 +0200 Subject: [PATCH] fix: handle boolean workflow inputs correctly before jobparser evaluates with them (#12539) Fixes https://code.forgejo.org/forgejo/forgejo-actions-feature-requests/issues/112. Currently boolean `workflow_dispatch` values are being passed as strings during Forgejo's job parsing, causing both true & false to have the same behaviour when evaluated in a condition like this: ``` on: workflow_dispatch: inputs: win32: type: boolean jobs: job1: strategy: matrix: runner: ${{ fromJSON(inputs.win32 == 'true' && '["win32", "win64"]' || '["win64"]') }} steps: # ... ``` ## 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... - [ ] 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... - [ ] `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/12539 Reviewed-by: Andreas Ahlenstorf --- services/actions/workflows.go | 8 ++++ tests/integration/actions_trigger_test.go | 58 +++++++++++++++++++++++ 2 files changed, 66 insertions(+) diff --git a/services/actions/workflows.go b/services/actions/workflows.go index a08a069cc1..d7d0f781d6 100644 --- a/services/actions/workflows.go +++ b/services/actions/workflows.go @@ -97,6 +97,8 @@ func (entry *Workflow) Dispatch(ctx context.Context, inputGetter InputValueGette title = fullWorkflowID } + // Runner expects a `map[string]string` for inputs in in the payload dispatch, but newer code in the Runner's + // jobparser library takes a map[string]any which is more directly actionable for parsing: inputs := make(map[string]string) inputsAny := make(map[string]any) if workflowDispatch := wf.WorkflowDispatchConfig(); workflowDispatch != nil { @@ -109,6 +111,12 @@ func (entry *Workflow) Dispatch(ctx context.Context, inputGetter InputValueGette } inputs[key] = value inputsAny[key] = value + // To match the behaviour of the runner when parsing map[string]string into map[string]any, check for + // boolean type inputs and convert them to booleans for expression evaluation: + // https://code.forgejo.org/forgejo/runner/src/commit/d5693e379c034a3afcb920087570d9a6e179e86e/act/runner/expression.go#L435-L439 + if input.Type == "boolean" { + inputsAny[key] = value == "true" + } } } diff --git a/tests/integration/actions_trigger_test.go b/tests/integration/actions_trigger_test.go index f5898682ce..19edb8b4ad 100644 --- a/tests/integration/actions_trigger_test.go +++ b/tests/integration/actions_trigger_test.go @@ -1009,6 +1009,64 @@ func TestActionsWorkflowDispatchDynamicMatrix(t *testing.T) { }) } +// Early in the job parsing, dynamic matrices may need to access workflow inputs. Boolean inputs need special handling +// which is what this test case covers. +func TestActionsWorkflowDispatchDynamicMatrixBooleanInput(t *testing.T) { + onApplicationRun(t, func(t *testing.T, u *url.URL) { + user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + + // create the repo + repo, sha, f := tests.CreateDeclarativeRepo(t, user2, "repo-workflow-dispatch", + []unit_model.Type{unit_model.TypeActions}, nil, + []*files_service.ChangeRepoFile{ + { + Operation: "create", + TreePath: ".forgejo/workflows/dispatch.yml", + ContentReader: strings.NewReader( + "name: test\n" + + "on:\n" + + " workflow_dispatch:\n" + + " inputs:\n" + + " win32:\n" + + " description: 'Boolean'\n" + + " required: false\n" + + " type: boolean\n" + + "jobs:\n" + + " test:\n" + + " runs-on: ubuntu-latest\n" + + " strategy:\n" + + " matrix:\n" + + " runner: ${{ fromJSON(inputs.win32 && '[\"win32\", \"win64\"]' || '[\"win64\"]') }}\n" + + " steps:\n" + + " - run: echo helloworld\n", + ), + }, + }, + ) + defer f() + + gitRepo, err := gitrepo.OpenRepository(db.DefaultContext, repo) + require.NoError(t, err) + defer gitRepo.Close() + + workflow, err := actions_service.GetWorkflowFromCommit(gitRepo, "main", "dispatch.yml") + require.NoError(t, err) + assert.Equal(t, "refs/heads/main", workflow.Ref) + assert.Equal(t, sha, workflow.Commit.ID.String()) + + inputGetter := func(key string) string { + return "false" + } + + run, _, err := workflow.Dispatch(db.DefaultContext, inputGetter, repo, user2) + require.NoError(t, err) + + jobs, err := actions_model.GetRunJobsByRunID(t.Context(), run.ID) + require.NoError(t, err) + assert.Len(t, jobs, 1) + }) +} + func TestActionsWorkflowDispatchReusableWorkflow(t *testing.T) { onApplicationRun(t, func(t *testing.T, u *url.URL) { user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})