From 3dbed63f55def83ab74f16de9387ce07d9811630 Mon Sep 17 00:00:00 2001 From: bircni Date: Mon, 26 Oct 2026 17:42:16 +0000 Subject: [PATCH] fix: close the action header group before the action runs (#1345) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `uses:` steps and job hooks kept their "Run …" group open until they finished. Gitea doesn't nest groups, so the action's own first `::group::` closed the header and everything before it stayed hidden there, e.g. all of goreleaser's output in https://gitea.com/gitea/cli/actions/runs/1121069/jobs/1519275. The header group now closes right after its details, as in actions/runner. The action header also matches actions/runner's otherwise now: it is titled by the `uses:` reference, lists the resolved inputs including `action.yml` defaults without empty ones, is shown for pre stages too, and post stages start with `Post job cleanup.`. --------- Co-authored-by: Lunny Xiao Co-authored-by: silverwind Reviewed-on: https://gitea.com/gitea/runner/pulls/1345 Reviewed-by: silverwind <2021+silverwind@noreply.gitea.com> Reviewed-by: Lunny Xiao Co-authored-by: bircni --- act/runner/action.go | 11 ++++++++++- act/runner/action_composite.go | 2 +- act/runner/action_composite_test.go | 5 +++-- act/runner/action_test.go | 9 +++++---- act/runner/job_hooks.go | 4 ++-- act/runner/job_hooks_test.go | 18 ++++++++++++++++-- act/runner/step_action_builtin.go | 4 +--- act/runner/step_action_local.go | 4 ---- act/runner/step_action_remote.go | 5 +---- act/runner/step_run.go | 24 +++++++++++++++++------- act/runner/step_run_print_test.go | 12 +++++++++--- docs/job-hooks.md | 2 +- 12 files changed, 66 insertions(+), 34 deletions(-) diff --git a/act/runner/action.go b/act/runner/action.go index 4599a8c4..b7d3e834 100644 --- a/act/runner/action.go +++ b/act/runner/action.go @@ -186,6 +186,7 @@ func runActionImpl(step actionStep, actionDir string, remoteAction *remoteAction return err } warnUnexpectedInputs(ctx, action, stepModel.With) + printRunActionHeader(ctx, stepModel, action, *step.getEnv(), rc) actionName, containerActionDir := getContainerActionPaths(step) @@ -504,10 +505,14 @@ func populateEnvsFromSavedState(env *map[string]string, step actionStep, rc *Run } } +func inputEnvKey(inputID string) string { + return "INPUT_" + inputEnvPattern.ReplaceAllString(strings.ToUpper(inputID), "_") +} + func populateEnvsFromInput(ctx context.Context, env *map[string]string, action *model.Action, rc *RunContext) error { eval := rc.NewExpressionEvaluator(ctx) for inputID, input := range action.Inputs { - envKey := "INPUT_" + inputEnvPattern.ReplaceAllString(strings.ToUpper(inputID), "_") + envKey := inputEnvKey(inputID) if _, ok := (*env)[envKey]; !ok { var err error if (*env)[envKey], err = eval.Interpolate(ctx, input.Default); err != nil { @@ -651,6 +656,7 @@ func runPreStep(step actionStep) common.Executor { return err } } + printRunActionHeader(ctx, step.getStepModel(), action, *step.getEnv(), rc) switch { case x.IsNode(): if err := maybeCopyToActionDir(ctx, step, actionDir, actionPath, containerActionDir); err != nil { @@ -749,6 +755,9 @@ func runPostStep(step actionStep) common.Executor { actionDir, actionPath, _, containerActionDir := actionStagePaths(step) x := action.Runs.Using + if !x.IsComposite() { // a composite's children print their own + logger.With(rawOutputField, true).Info("Post job cleanup.") + } switch { case x.IsNode(): diff --git a/act/runner/action_composite.go b/act/runner/action_composite.go index a36b3a7c..3b3eb3b5 100644 --- a/act/runner/action_composite.go +++ b/act/runner/action_composite.go @@ -31,7 +31,7 @@ func evaluateCompositeInputAndEnv(ctx context.Context, parent *RunContext, step ee := parent.NewStepExpressionEvaluator(ctx, step) for inputID, input := range step.getActionModel().Inputs { - envKey := "INPUT_" + inputEnvPattern.ReplaceAllString(strings.ToUpper(inputID), "_") + envKey := inputEnvKey(inputID) // lookup if key is defined in the step but the already // evaluated value from the environment diff --git a/act/runner/action_composite_test.go b/act/runner/action_composite_test.go index b70b135e..9792564f 100644 --- a/act/runner/action_composite_test.go +++ b/act/runner/action_composite_test.go @@ -9,6 +9,7 @@ import ( "gitea.com/gitea/runner/act/common" "gitea.com/gitea/runner/act/common/git" + "gitea.com/gitea/runner/act/common/logging" "gitea.com/gitea/runner/act/common/logging/logtest" "gitea.dev/actionslib/pkg/model" @@ -19,8 +20,8 @@ import ( func TestCompositeActionParity(t *testing.T) { t.Run("inherits contexts without leaking inputs", func(t *testing.T) { - logger, hook := logtest.NewLogger() - ctx := common.WithLogger(t.Context(), logger) + hook := logtest.NewWithLevel(logging.LevelWarn) + ctx := common.WithLogger(t.Context(), logging.New(hook)) strategy := &model.Strategy{MaxParallelString: "3", RawMatrix: yaml.Node{Kind: yaml.MappingNode}} parent := &RunContext{ Config: &Config{PresetGitHubContext: &model.GithubContext{}}, diff --git a/act/runner/action_test.go b/act/runner/action_test.go index aeba8438..1f919492 100644 --- a/act/runner/action_test.go +++ b/act/runner/action_test.go @@ -20,6 +20,7 @@ import ( "gitea.com/gitea/runner/act/common" "gitea.com/gitea/runner/act/common/git" + "gitea.com/gitea/runner/act/common/logging" "gitea.com/gitea/runner/act/common/logging/logtest" "gitea.com/gitea/runner/act/container" @@ -197,8 +198,8 @@ func TestActionRunner(t *testing.T) { }, } { t.Run(tt.name, func(t *testing.T) { - logger, hook := logtest.NewLogger() - ctx := common.WithLogger(t.Context(), logger) + hook := logtest.NewWithLevel(logging.LevelWarn) + ctx := common.WithLogger(t.Context(), logging.New(hook)) cm := &containerMock{} containerActionDir := "/var/run/act/actions/" + model.UsesHash("/org/repo/path@ref") + "/" cm.On("CopyDir", containerActionDir, "dir/", false, true).Return(func(_ctx context.Context) error { return nil }) @@ -664,8 +665,8 @@ func TestExecAsDockerStageEntrypoint(t *testing.T) { }, } { t.Run(tc.name, func(t *testing.T) { - logger, hook := logtest.NewLogger() - ctx := common.WithLogger(t.Context(), logger) + hook := logtest.NewWithLevel(logging.LevelWarn) + ctx := common.WithLogger(t.Context(), logging.New(hook)) tc.runs.Using, tc.runs.Image = "docker", "docker://node:14" cm := &containerMock{} var input *container.NewContainerInput diff --git a/act/runner/job_hooks.go b/act/runner/job_hooks.go index 26ae6387..246353b6 100644 --- a/act/runner/job_hooks.go +++ b/act/runner/job_hooks.go @@ -46,12 +46,12 @@ func (rc *RunContext) runJobHook(ctx context.Context, hookPath, name string) err cmd, shell := hookCommand(hookPath) rawLogger := common.Logger(ctx).With(rawOutputField, true) - defer rawLogger.Infof("::endgroup::") - rawLogger.Infof("::group::Run '%s'", EscapeCommandData(hookPath)) rawLogger.Infof("A %s hook has been configured by the runner administrator", name) + rawLogger.Infof("::group::Run '%s'", EscapeCommandData(hookPath)) if shell != "" { rawLogger.Infof("shell: %s", shell) } + rawLogger.Infof("::endgroup::") env := mergeMaps(rc.containerSpec.Env, rc.GetEnv()) rc.withGithubEnv(ctx, rc.getGithubContext(ctx), env) diff --git a/act/runner/job_hooks_test.go b/act/runner/job_hooks_test.go index 94e81eeb..dff4a4e6 100644 --- a/act/runner/job_hooks_test.go +++ b/act/runner/job_hooks_test.go @@ -91,15 +91,29 @@ func newHookRunContext(jobContainer *hookContainer, config *Config) (*RunContext } func TestRunJobHook(t *testing.T) { - t.Run("runs the hook with the job environment", func(t *testing.T) { - jobContainer := &hookContainer{} + t.Run("runs the hook with the job environment after its closed header group", func(t *testing.T) { + jobContainer := &hookContainer{output: "from the hook\n"} rc, ctx := newHookRunContext(jobContainer, &Config{ JobStartedHook: "/hooks/started.sh", Env: map[string]string{"A_VAR": "value", jobStartedHookEnv: "/from/env.sh"}, }) + records := logtest.New() + ctx = common.WithLogger(ctx, logging.New(records)) require.NoError(t, rc.runJobStartedHook(ctx)) + var lines []string + for _, record := range records.AllRecords() { + lines = append(lines, record.Message) + } + assert.Equal(t, []string{ + "A job started hook has been configured by the runner administrator", + "::group::Run '/hooks/started.sh'", + "shell: bash -e {0}", + "::endgroup::", + "from the hook\n", + }, lines) + // The setting wins over the environment variable. assert.Equal(t, []string{"bash", "-e", "/hooks/started.sh"}, jobContainer.cmd) assert.Equal(t, "value", jobContainer.env["A_VAR"]) diff --git a/act/runner/step_action_builtin.go b/act/runner/step_action_builtin.go index f0943790..c3442125 100644 --- a/act/runner/step_action_builtin.go +++ b/act/runner/step_action_builtin.go @@ -33,9 +33,7 @@ func (sab *stepActionBuiltin) main() common.Executor { return nil } - printRunActionHeader(ctx, sab.Step, sab.env, sab.RunContext) - rawLogger := common.Logger(ctx).With(rawOutputField, true) - defer rawLogger.Infof("::endgroup::") + printRunActionHeader(ctx, sab.Step, nil, sab.env, sab.RunContext) rc := sab.RunContext rc.ApplyExtraPath(ctx, &sab.env) diff --git a/act/runner/step_action_local.go b/act/runner/step_action_local.go index ad02e981..0fde48c3 100644 --- a/act/runner/step_action_local.go +++ b/act/runner/step_action_local.go @@ -42,10 +42,6 @@ func (sal *stepActionLocal) main() common.Executor { return nil } - printRunActionHeader(ctx, sal.Step, sal.env, sal.getRunContext()) - rawLogger := common.Logger(ctx).With(rawOutputField, true) - defer rawLogger.Infof("::endgroup::") - actionDir := filepath.Join(sal.getRunContext().Config.Workdir, sal.Step.Uses) _, containerActionPath := getContainerActionPaths(sal) diff --git a/act/runner/step_action_remote.go b/act/runner/step_action_remote.go index 4a504fb2..31ded4ca 100644 --- a/act/runner/step_action_remote.go +++ b/act/runner/step_action_remote.go @@ -153,12 +153,9 @@ func (sar *stepActionRemote) main() common.Executor { return common.NewPipelineExecutor( sar.prepareActionExecutor(), runStepExecutor(sar, stepStageMain, func(ctx context.Context) error { - printRunActionHeader(ctx, sar.Step, sar.env, sar.RunContext) - rawLogger := common.Logger(ctx).With(rawOutputField, true) - defer rawLogger.Infof("::endgroup::") - github := sar.getGithubContext(ctx) if sar.skipsLocalCheckout(sar.remoteAction, github) { + printRunActionHeader(ctx, sar.Step, nil, sar.env, sar.RunContext) if sar.RunContext.Config.BindWorkdir { common.Logger(ctx).Debugf("Skipping local actions/checkout because you bound your workspace") return nil diff --git a/act/runner/step_run.go b/act/runner/step_run.go index f8f174d4..29e0565c 100644 --- a/act/runner/step_run.go +++ b/act/runner/step_run.go @@ -87,24 +87,34 @@ func (sr *stepRun) printRunScriptActionDetails(ctx context.Context) { } // printRunActionHeader mirrors actions/runner's "Run " header for `uses:` steps, -// including the with: inputs and the step-level env: block. The caller is responsible -// for emitting ::endgroup:: after the action finishes. -func printRunActionHeader(ctx context.Context, step *model.Step, env map[string]string, rc *RunContext) { +// including the with: inputs and the step-level env: block. +func printRunActionHeader(ctx context.Context, step *model.Step, action *model.Action, env map[string]string, rc *RunContext) { if step == nil { return } rawLogger := common.Logger(ctx).With(rawOutputField, true) - rawLogger.Infof("::group::Run %s", EscapeCommandData(cmp.Or(step.Name, step.Uses))) + rawLogger.Infof("::group::Run %s", EscapeCommandData(step.Uses)) - if len(step.With) > 0 { + inputs := map[string]string{} + maps.Copy(inputs, step.With) + if action != nil { + for name := range action.Inputs { + if _, provided := lookupFold(step.With, name, true); !provided { + inputs[name] = env[inputEnvKey(name)] + } + } + } + maps.DeleteFunc(inputs, func(_name, value string) bool { return value == "" }) + if len(inputs) > 0 { rawLogger.Infof("with:") - for _, k := range slices.Sorted(maps.Keys(step.With)) { - logKeyedValue(rawLogger, k, step.With[k]) + for _, k := range slices.Sorted(maps.Keys(inputs)) { + logKeyedValue(rawLogger, k, inputs[k]) } } printStepEnvBlock(ctx, step, env, rc) + rawLogger.Infof("::endgroup::") } // printStepEnvBlock emits the declared-env block (YAML order, internal vars filtered) diff --git a/act/runner/step_run_print_test.go b/act/runner/step_run_print_test.go index e0b2ec54..a63a53d2 100644 --- a/act/runner/step_run_print_test.go +++ b/act/runner/step_run_print_test.go @@ -118,10 +118,12 @@ env: func TestPrintRunActionHeaderGolden(t *testing.T) { raw := `id: s1 +name: Checkout uses: actions/checkout@v4 with: fetch-depth: "0" - token: secret + Token: secret + clean: "" env: CUSTOM: value GITHUB_TOKEN: tok @@ -134,15 +136,19 @@ env: entry := logger.With("job", "j1") ctx := common.WithLogger(t.Context(), entry) - printRunActionHeader(ctx, &step, map[string]string{"CUSTOM": "value", "GITHUB_TOKEN": "tok"}, &RunContext{}) + action := &model.Action{Inputs: map[string]model.Input{"token": {}, "clean": {}, "path": {}, "lfs": {}}} + env := map[string]string{"CUSTOM": "value", "GITHUB_TOKEN": "tok", "INPUT_TOKEN": "secret", "INPUT_CLEAN": "", "INPUT_PATH": "sub"} + printRunActionHeader(ctx, &step, action, env, &RunContext{}) want := strings.Join([]string{ "[j1] | ::group::Run actions/checkout@v4", "[j1] | with:", + "[j1] | Token: secret", "[j1] | fetch-depth: 0", - "[j1] | token: secret", + "[j1] | path: sub", "[j1] | env:", "[j1] | CUSTOM: value", + "[j1] | ::endgroup::", "", }, "\n") assert.Equal(t, want, buf.String()) diff --git a/docs/job-hooks.md b/docs/job-hooks.md index f7a4080a..1cd43bff 100644 --- a/docs/job-hooks.md +++ b/docs/job-hooks.md @@ -47,7 +47,7 @@ A hook path that does not exist inside the job environment fails the job with `N A hook sees the job's environment: the workflow, job and `container:` `env:`, the runner's `envs`, and the `GITHUB_*` context variables, with the same masking applied to its output as to a step's. The step-specific ones (`GITHUB_ACTION`, `GITHUB_OUTPUT`, `GITHUB_STATE`) are not set — a hook is not a step, so `::save-state::` and `::set-output::` have nowhere to go. -Its stdout is part of the job log, inside a collapsible group, and is scanned for workflow commands. `::add-mask::` registers a value to be masked for the rest of the job. `::set-env::` and `::add-path::` apply to the steps that follow, but as in a step they need `ACTIONS_ALLOW_UNSECURE_COMMANDS: true`, and a refused one fails the hook. +Its stdout is part of the job log, after a collapsible group naming the hook and its shell, and is scanned for workflow commands. `::add-mask::` registers a value to be masked for the rest of the job. `::set-env::` and `::add-path::` apply to the steps that follow, but as in a step they need `ACTIONS_ALLOW_UNSECURE_COMMANDS: true`, and a refused one fails the hook. `$GITHUB_ENV` and `$GITHUB_PATH` point at files that are read back after the hook exits, so the file-command form works too: