From 2ea5a8d22bee181cd2c01dd708a700bf68e0f23b Mon Sep 17 00:00:00 2001
From: Mathieu Fenniak <mathieu@fenniak.net>
Date: Sat, 15 Nov 2025 12:58:48 +0100
Subject: [PATCH] fix: workflow dispatch shouldn't include empty fields in
 inputs (#10123)

Fix behaviour change from #10089.  Empty inputs used to hit a `continue` statement and skip, and are now fired to a workflow.  It isn't likely this is a functional bug, but it does change the behaviour unexpectedly.

Detected by end-to-end test failure (https://code.forgejo.org/forgejo/end-to-end/actions/runs/4360/jobs/2/attempt/1):
```
 {
-  number2: ""
-  tags: ""
 }
```

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/10123
Reviewed-by: Michael Kriese <michael.kriese@gmx.de>
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net>
Co-committed-by: Mathieu Fenniak <mathieu@fenniak.net>
---
 services/actions/workflows.go      |  7 +++++-
 services/actions/workflows_test.go | 34 ++++++++++++++++++++++++------
 2 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/services/actions/workflows.go b/services/actions/workflows.go
index 18cf92e64d..e090e3f396 100644
--- a/services/actions/workflows.go
+++ b/services/actions/workflows.go
@@ -49,6 +49,8 @@ type Workflow struct {
 
 type InputValueGetter func(key string) string
 
+var ErrSkipDispatchInput = errors.New("skip dispatching of input")
+
 func resolveDispatchInput(key, value string, input act_model.WorkflowDispatchInput) (string, error) {
 	if len(value) == 0 {
 		value = input.Default
@@ -60,6 +62,7 @@ func resolveDispatchInput(key, value string, input act_model.WorkflowDispatchInp
 				}
 				return "", InputRequiredErr{Name: name}
 			}
+			return "", ErrSkipDispatchInput
 		}
 	} else if input.Type == "boolean" {
 		// Temporary compatibility shim for people that upgrade to Forgejo 14. Can be removed with Forgejo 15.
@@ -94,7 +97,9 @@ func (entry *Workflow) Dispatch(ctx context.Context, inputGetter InputValueGette
 	if workflowDispatch := wf.WorkflowDispatchConfig(); workflowDispatch != nil {
 		for key, input := range workflowDispatch.Inputs {
 			value, err := resolveDispatchInput(key, inputGetter(key), input)
-			if err != nil {
+			if err == ErrSkipDispatchInput {
+				continue
+			} else if err != nil {
 				return nil, nil, err
 			}
 			inputs[key] = value
diff --git a/services/actions/workflows_test.go b/services/actions/workflows_test.go
index 555308018d..138965e0fd 100644
--- a/services/actions/workflows_test.go
+++ b/services/actions/workflows_test.go
@@ -4,6 +4,7 @@
 package actions
 
 import (
+	"errors"
 	"testing"
 
 	actions_model "forgejo.org/models/actions"
@@ -124,11 +125,12 @@ func TestConfigureActionRunConcurrency(t *testing.T) {
 
 func TestResolveDispatchInputAcceptsValidInput(t *testing.T) {
 	for _, tc := range []struct {
-		name     string
-		key      string
-		value    string
-		input    act_model.WorkflowDispatchInput
-		expected string
+		name          string
+		key           string
+		value         string
+		input         act_model.WorkflowDispatchInput
+		expected      string
+		expectedError func(err error) bool
 	}{
 		{
 			name:     "on_converted_to_true",
@@ -202,10 +204,28 @@ func TestResolveDispatchInputAcceptsValidInput(t *testing.T) {
 			input:    act_model.WorkflowDispatchInput{Description: "a string", Required: false, Type: "number", Options: []string{}},
 			expected: "123",
 		},
+		{
+			name:          "empty_value_skipped",
+			key:           "my_number",
+			value:         "",
+			input:         act_model.WorkflowDispatchInput{Description: "a string", Required: false, Type: "number", Options: []string{}},
+			expectedError: func(err error) bool { return errors.Is(err, ErrSkipDispatchInput) },
+		},
+		{
+			name:          "required_missing",
+			key:           "my_number",
+			value:         "",
+			input:         act_model.WorkflowDispatchInput{Required: true, Type: "number", Options: []string{}},
+			expectedError: IsInputRequiredErr,
+		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
-			actual, _ := resolveDispatchInput(tc.key, tc.value, tc.input)
-			assert.Equal(t, tc.expected, actual)
+			actual, err := resolveDispatchInput(tc.key, tc.value, tc.input)
+			if tc.expectedError != nil {
+				assert.True(t, tc.expectedError(err))
+			} else {
+				assert.Equal(t, tc.expected, actual)
+			}
 		})
 	}
 }