chore(lint): add golangci-lint v2 and fix all lint issues (#803)

## Summary
- Replace old `.golangci.yml` (v1 format) with v2 format, aligned with gitea's lint config
- Add `lint-go`, `lint-go-fix`, and `lint` Makefile targets using golangci-lint v2.10.1
- Replace `make vet` with `make lint` in CI workflow (lint includes vet)
- Fix all 35 lint issues: modernize (maps.Copy, range over int, any), perfsprint (errors.New), unparam (remove unused parameters), revive (var naming), staticcheck, forbidigo exclusion for cmd/
- Make `security-check` non-fatal (apply https://github.com/go-gitea/gitea/pull/36681)
- Remove dead gocritic exclusion rules (commentFormatting, exitAfterDefer)
- Remove dead linter exclusions and disabled checks (singleCaseSwitch, ST1003, QF1001, QF1006, QF1008, testifylint go-require/require-error, test file exclusions for dupl/errcheck/staticcheck/unparam)

## Test plan
- [x] `golangci-lint run` passes
- [x] `go build ./...` passes
- [x] `go test ./...` passes

---------

Co-authored-by: ChristopherHX <christopher.homberger@web.de>
Co-authored-by: Christopher Homberger <christopher.homberger@web.de>
Reviewed-on: https://gitea.com/gitea/act_runner/pulls/803
Reviewed-by: ChristopherHX <christopherhx@noreply.gitea.com>

.gitea/workflows/test.yml

@@ -12,8 +12,8 @@ jobs:
       - uses: actions/setup-go@v6
         with:
           go-version-file: 'go.mod'
-      - name: vet checks
+      - name: lint
-        run: make vet
+        run: make lint
       - name: build
         run: make build
       - name: test

.golangci.yml

@@ -1,165 +1,112 @@
+version: "2"
+output:
+  sort-order:
+    - file
 linters:
+  default: none
   enable:
-    - gosimple
-    - typecheck
-    - govet
-    - errcheck
-    - staticcheck
-    - unused
-    - dupl
-    #- gocyclo # The cyclomatic complexety of a lot of functions is too high, we should refactor those another time.
-    - gofmt
-    - misspell
-    - gocritic
     - bidichk
-    - ineffassign
+    - bodyclose
-    - revive
-    - gofumpt
     - depguard
+    - dupl
+    - errcheck
+    - forbidigo
+    - gocheckcompilerdirectives
+    - gocritic
+    - govet
+    - ineffassign
+    - mirror
+    - modernize
     - nakedret
-    - unconvert
+    - nilnil
-    - wastedassign
     - nolintlint
-    - stylecheck
+    - perfsprint
-  enable-all: false
+    - revive
-  disable-all: true
+    - staticcheck
-  fast: false
+    - testifylint
-
+    - unconvert
-run:
+    - unparam
-  go: 1.18
+    - unused
-  timeout: 10m
+    - usestdlibvars
-  skip-dirs:
+    - usetesting
-    - node_modules
+    - wastedassign
-    - public
+  settings:
-    - web_src
+    depguard:
-
+      rules:
-linters-settings:
+        main:
-  stylecheck:
+          deny:
-    checks: ["all", "-ST1005", "-ST1003"]
+            - pkg: io/ioutil
-  nakedret:
+              desc: use os or io instead
-    max-func-lines: 0
+            - pkg: golang.org/x/exp
-  gocritic:
+              desc: it's experimental and unreliable
-    disabled-checks:
+            - pkg: github.com/pkg/errors
-      - ifElseChain
+              desc: use builtin errors package instead
-      - singleCaseSwitch # Every time this occurred in the code, there  was no other way.
+    nolintlint:
-  revive:
+      allow-unused: false
-    ignore-generated-header: false
+      require-explanation: true
-    severity: warning
+      require-specific: true
-    confidence: 0.8
+    gocritic:
-    errorCode: 1
+      enabled-checks:
-    warningCode: 1
+        - equalFold
+      disabled-checks:
+        - ifElseChain
+    revive:
+      severity: error
+      rules:
+        - name: blank-imports
+        - name: constant-logical-expr
+        - name: context-as-argument
+        - name: context-keys-type
+        - name: dot-imports
+        - name: empty-lines
+        - name: error-return
+        - name: error-strings
+        - name: exported
+        - name: identical-branches
+        - name: if-return
+        - name: increment-decrement
+        - name: modifies-value-receiver
+        - name: package-comments
+        - name: redefines-builtin-id
+        - name: superfluous-else
+        - name: time-naming
+        - name: unexported-return
+        - name: var-declaration
+        - name: var-naming
+    staticcheck:
+      checks:
+        - all
+        - -ST1005
+    usetesting:
+      os-temp-dir: true
+    perfsprint:
+      concat-loop: false
+    govet:
+      enable:
+        - nilness
+        - unusedwrite
+  exclusions:
+    generated: lax
+    presets:
+      - comments
+      - common-false-positives
+      - legacy
+      - std-error-handling
     rules:
-      - name: blank-imports
+      - linters:
-      - name: context-as-argument
+          - forbidigo
-      - name: context-keys-type
+        path: cmd
-      - name: dot-imports
-      - name: error-return
-      - name: error-strings
-      - name: error-naming
-      - name: exported
-      - name: if-return
-      - name: increment-decrement
-      - name: var-naming
-      - name: var-declaration
-      - name: package-comments
-      - name: range
-      - name: receiver-naming
-      - name: time-naming
-      - name: unexported-return
-      - name: indent-error-flow
-      - name: errorf
-      - name: duplicated-imports
-      - name: modifies-value-receiver
-  gofumpt:
-    extra-rules: true
-    lang-version: "1.18"
-  depguard:
-    # TODO: use depguard to replace import checks in gitea-vet
-    list-type: denylist
-    # Check the list against standard lib.
-    include-go-root: true
-    packages-with-error-message:
-      - github.com/unknwon/com: "use gitea's util and replacements"
-
 issues:
-  exclude-rules:
+  max-issues-per-linter: 0
-    # Exclude some linters from running on tests files.
+  max-same-issues: 0
-    - path: _test\.go
+formatters:
-      linters:
+  enable:
-        - gocyclo
+    - gofmt
-        - errcheck
+    - gofumpt
-        - dupl
+  settings:
-        - gosec
+    gofumpt:
-        - unparam
+      extra-rules: true
-        - staticcheck
+  exclusions:
-    - path: models/migrations/v
+    generated: lax
-      linters:
+run:
-        - gocyclo
+  timeout: 10m
-        - errcheck
-        - dupl
-        - gosec
-    - linters:
-        - dupl
-      text: "webhook"
-    - linters:
-        - gocritic
-      text: "`ID' should not be capitalized"
-    - path: modules/templates/helper.go
-      linters:
-        - gocritic
-    - linters:
-        - unused
-      text: "swagger"
-    - path: contrib/pr/checkout.go
-      linters:
-        - errcheck
-    - path: models/issue.go
-      linters:
-        - errcheck
-    - path: models/migrations/
-      linters:
-        - errcheck
-    - path: modules/log/
-      linters:
-        - errcheck
-    - path: routers/api/v1/repo/issue_subscription.go
-      linters:
-        - dupl
-    - path: routers/repo/view.go
-      linters:
-        - dupl
-    - path: models/migrations/
-      linters:
-        - unused
-    - linters:
-        - staticcheck
-      text: "argument x is overwritten before first use"
-    - path: modules/httplib/httplib.go
-      linters:
-        - staticcheck
-    # Enabling this would require refactoring the methods and how they are called.
-    - path: models/issue_comment_list.go
-      linters:
-        - dupl
-    - linters:
-        - misspell
-      text: '`Unknwon` is a misspelling of `Unknown`'
-    - path: models/update.go
-      linters:
-        - unused
-    - path: cmd/dump.go
-      linters:
-        - dupl
-    - text: "commentFormatting: put a space between `//` and comment text"
-      linters:
-        - gocritic
-    - text: "exitAfterDefer:"
-      linters:
-        - gocritic
-    - path: modules/graceful/manager_windows.go
-      linters:
-        - staticcheck
-      text: "svc.IsAnInteractiveSession is deprecated: Use IsWindowsService instead."
-    - path: models/user/openid.go
-      linters:
-        - golint

Makefile

@@ -20,6 +20,7 @@ DOCKER_TAG ?= nightly
 DOCKER_REF := $(DOCKER_IMAGE):$(DOCKER_TAG)
 DOCKER_ROOTLESS_REF := $(DOCKER_IMAGE):$(DOCKER_TAG)-dind-rootless
 
+GOLANGCI_LINT_PACKAGE ?= github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.10.1
 GOVULNCHECK_PACKAGE ?= golang.org/x/vuln/cmd/govulncheck@v1
 
 ifneq ($(shell uname), Darwin)
@@ -107,9 +108,20 @@ fmt-check:
 deps-tools: ## install tool dependencies
 	$(GO) install $(GOVULNCHECK_PACKAGE)
 
+.PHONY: lint
+lint: lint-go vet
+
+.PHONY: lint-go
+lint-go: ## lint go files
+	$(GO) run $(GOLANGCI_LINT_PACKAGE) run
+
+.PHONY: lint-go-fix
+lint-go-fix: ## lint go files and fix issues
+	$(GO) run $(GOLANGCI_LINT_PACKAGE) run --fix
+
 .PHONY: security-check
 security-check: deps-tools
-	GOEXPERIMENT= $(GO) run $(GOVULNCHECK_PACKAGE) -show color ./...
+	GOEXPERIMENT= $(GO) run $(GOVULNCHECK_PACKAGE) -show color ./... || true
 
 .PHONY: tidy
 tidy:
@@ -125,7 +137,7 @@ tidy-check: tidy
 	fi
 
 test: fmt-check security-check
-	@$(GO) test -v -cover -coverprofile coverage.txt ./... && echo "\n==>\033[32m Ok\033[m\n" || exit 1
+	@$(GO) test -race -v -cover -coverprofile coverage.txt ./... && echo "\n==>\033[32m Ok\033[m\n" || exit 1
 
 .PHONY: vet
 vet:

internal/app/cmd/cache-server.go

@@ -4,7 +4,6 @@
 package cmd
 
 import (
-	"context"
 	"fmt"
 	"os"
 	"os/signal"
@@ -22,7 +21,7 @@ type cacheServerArgs struct {
 	Port uint16
 }
 
-func runCacheServer(ctx context.Context, configFile *string, cacheArgs *cacheServerArgs) func(cmd *cobra.Command, args []string) error {
+func runCacheServer(configFile *string, cacheArgs *cacheServerArgs) func(cmd *cobra.Command, args []string) error {
 	return func(cmd *cobra.Command, args []string) error {
 		cfg, err := config.LoadDefault(*configFile)
 		if err != nil {

internal/app/cmd/cmd.go

@@ -72,7 +72,7 @@ func Execute(ctx context.Context) {
 		Use:   "cache-server",
 		Short: "Start a cache server for the cache action",
 		Args:  cobra.MaximumNArgs(0),
-		RunE:  runCacheServer(ctx, &configFile, &cacheArgs),
+		RunE:  runCacheServer(&configFile, &cacheArgs),
 	}
 	cacheCmd.Flags().StringVarP(&cacheArgs.Dir, "dir", "d", "", "Cache directory")
 	cacheCmd.Flags().StringVarP(&cacheArgs.Host, "host", "s", "", "Host of the cache server")

internal/app/cmd/daemon.go

@@ -262,5 +262,5 @@ func getDockerSocketPath(configDockerHost string) (string, error) {
 		}
 	}
 
-	return "", fmt.Errorf("daemon Docker Engine socket not found and docker_host config was invalid")
+	return "", errors.New("daemon Docker Engine socket not found and docker_host config was invalid")
 }

internal/app/cmd/exec.go

@@ -6,7 +6,9 @@ package cmd
 
 import (
 	"context"
+	"errors"
 	"fmt"
+	"maps"
 	"os"
 	"path/filepath"
 	"strconv"
@@ -77,7 +79,7 @@ func (i *executeArgs) LoadSecrets() map[string]string {
 	for _, secretPair := range i.secrets {
 		secretPairParts := strings.SplitN(secretPair, "=", 2)
 		secretPairParts[0] = strings.ToUpper(secretPairParts[0])
-		if strings.ToUpper(s[secretPairParts[0]]) == secretPairParts[0] {
+		if strings.EqualFold(s[secretPairParts[0]], secretPairParts[0]) {
 			log.Errorf("Secret %s is already defined (secrets are case insensitive)", secretPairParts[0])
 		}
 		if len(secretPairParts) == 2 {
@@ -104,9 +106,7 @@ func readEnvs(path string, envs map[string]string) bool {
 		if err != nil {
 			log.Fatalf("Error loading from %s: %v", path, err)
 		}
-		for k, v := range env {
+		maps.Copy(envs, env)
-			envs[k] = v
-		}
 		return true
 	}
 	return false
@@ -166,7 +166,7 @@ func (i *executeArgs) resolve(path string) string {
 	return path
 }
 
-func printList(plan *model.Plan) error {
+func printList(plan *model.Plan) {
 	type lineInfoDef struct {
 		jobID   string
 		jobName string
@@ -261,10 +261,9 @@ func printList(plan *model.Plan) error {
 	if duplicateJobIDs {
 		fmt.Print("\nDetected multiple jobs with the same job name, use `-W` to specify the path to the specific workflow.\n")
 	}
-	return nil
 }
 
-func runExecList(ctx context.Context, planner model.WorkflowPlanner, execArgs *executeArgs) error {
+func runExecList(planner model.WorkflowPlanner, execArgs *executeArgs) error {
 	// plan with filtered jobs - to be used for filtering only
 	var filterPlan *model.Plan
 
@@ -306,7 +305,7 @@ func runExecList(ctx context.Context, planner model.WorkflowPlanner, execArgs *e
 		}
 	}
 
-	_ = printList(filterPlan)
+	printList(filterPlan)
 
 	return nil
 }
@@ -319,7 +318,7 @@ func runExec(ctx context.Context, execArgs *executeArgs) func(cmd *cobra.Command
 		}
 
 		if execArgs.runList {
-			return runExecList(ctx, planner, execArgs)
+			return runExecList(planner, execArgs)
 		}
 
 		// plan with triggered jobs
@@ -378,7 +377,7 @@ func runExec(ctx context.Context, execArgs *executeArgs) func(cmd *cobra.Command
 		if len(execArgs.artifactServerAddr) == 0 {
 			ip := common.GetOutboundIP()
 			if ip == nil {
-				return fmt.Errorf("unable to determine outbound IP address")
+				return errors.New("unable to determine outbound IP address")
 			}
 			execArgs.artifactServerAddr = ip.String()
 		}
@@ -422,7 +421,7 @@ func runExec(ctx context.Context, execArgs *executeArgs) func(cmd *cobra.Command
 			NoSkipCheckout:        execArgs.noSkipCheckout,
 			// PresetGitHubContext:   preset,
 			// EventJSON:             string(eventJSON),
-			ContainerNamePrefix:   fmt.Sprintf("GITEA-ACTIONS-TASK-%s", eventName),
+			ContainerNamePrefix:   "GITEA-ACTIONS-TASK-" + eventName,
 			ContainerMaxLifetime:  maxLifetime,
 			ContainerNetworkMode:  container.NetworkMode(execArgs.network),
 			DefaultActionInstance: execArgs.defaultActionsURL,

internal/app/cmd/register.go

@@ -6,6 +6,7 @@ package cmd
 import (
 	"bufio"
 	"context"
+	"errors"
 	"fmt"
 	"os"
 	"os/signal"
@@ -107,10 +108,10 @@ type registerInputs struct {
 
 func (r *registerInputs) validate() error {
 	if r.InstanceAddr == "" {
-		return fmt.Errorf("instance address is empty")
+		return errors.New("instance address is empty")
 	}
 	if r.Token == "" {
-		return fmt.Errorf("token is empty")
+		return errors.New("token is empty")
 	}
 	if len(r.Labels) > 0 {
 		return validateLabels(r.Labels)

internal/app/poll/poller.go

@@ -102,7 +102,7 @@ func (p *Poller) Shutdown(ctx context.Context) error {
 		p.shutdownJobs()
 
 		// wait for running jobs to report their status to Gitea
-		_, _ = <-p.done
+		<-p.done
 
 		return ctx.Err()
 	}

internal/app/run/runner.go

@@ -7,6 +7,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"maps"
 	"path/filepath"
 	"strings"
 	"sync"
@@ -49,9 +50,7 @@ func NewRunner(cfg *config.Config, reg *config.Registration, cli client.Client)
 		}
 	}
 	envs := make(map[string]string, len(cfg.Runner.Envs))
-	for k, v := range cfg.Runner.Envs {
+	maps.Copy(envs, cfg.Runner.Envs)
-		envs[k] = v
-	}
 	if cfg.Cache.Enabled == nil || *cfg.Cache.Enabled {
 		if cfg.Cache.ExternalServer != "" {
 			envs["ACTIONS_CACHE_URL"] = cfg.Cache.ExternalServer
@@ -116,7 +115,7 @@ func (r *Runner) Run(ctx context.Context, task *runnerv1.Task) error {
 // getDefaultActionsURL
 // when DEFAULT_ACTIONS_URL == "https://github.com" and GithubMirror is not blank,
 // it should be set to GithubMirror first.
-func (r *Runner) getDefaultActionsURL(ctx context.Context, task *runnerv1.Task) string {
+func (r *Runner) getDefaultActionsURL(task *runnerv1.Task) string {
 	giteaDefaultActionsURL := task.Context.Fields["gitea_default_actions_url"].GetStringValue()
 	if giteaDefaultActionsURL == "https://github.com" && r.cfg.Runner.GithubMirror != "" {
 		return r.cfg.Runner.GithubMirror
@@ -148,7 +147,7 @@ func (r *Runner) run(ctx context.Context, task *runnerv1.Task, reporter *report.
 	taskContext := task.Context.Fields
 
 	log.Infof("task %v repo is %v %v %v", task.Id, taskContext["repository"].GetStringValue(),
-		r.getDefaultActionsURL(ctx, task),
+		r.getDefaultActionsURL(task),
 		r.client.Address())
 
 	preset := &model.GithubContext{
@@ -174,8 +173,8 @@ func (r *Runner) run(ctx context.Context, task *runnerv1.Task, reporter *report.
 		preset.Token = t
 	}
 
-	if actionsIdTokenRequestUrl := taskContext["actions_id_token_request_url"].GetStringValue(); actionsIdTokenRequestUrl != "" {
+	if actionsIDTokenRequestURL := taskContext["actions_id_token_request_url"].GetStringValue(); actionsIDTokenRequestURL != "" {
-		r.envs["ACTIONS_ID_TOKEN_REQUEST_URL"] = actionsIdTokenRequestUrl
+		r.envs["ACTIONS_ID_TOKEN_REQUEST_URL"] = actionsIDTokenRequestURL
 		r.envs["ACTIONS_ID_TOKEN_REQUEST_TOKEN"] = taskContext["actions_id_token_request_token"].GetStringValue()
 		task.Secrets["ACTIONS_ID_TOKEN_REQUEST_TOKEN"] = r.envs["ACTIONS_ID_TOKEN_REQUEST_TOKEN"]
 	}
@@ -222,7 +221,7 @@ func (r *Runner) run(ctx context.Context, task *runnerv1.Task, reporter *report.
 		ContainerOptions:      r.cfg.Container.Options,
 		ContainerDaemonSocket: r.cfg.Container.DockerHost,
 		Privileged:            r.cfg.Container.Privileged,
-		DefaultActionInstance: r.getDefaultActionsURL(ctx, task),
+		DefaultActionInstance: r.getDefaultActionsURL(task),
 		PlatformPicker:        r.labels.PickPlatform,
 		Vars:                  task.Vars,
 		ValidVolumes:          r.cfg.Container.ValidVolumes,

internal/app/run/workflow_test.go

@@ -307,7 +307,7 @@ func Test_yamlV4NodeRoundTrip(t *testing.T) {
 	t.Run("unmarshal and re-marshal workflow", func(t *testing.T) {
 		input := []byte("name: test\non: push\njobs:\n    build:\n        runs-on: ubuntu-latest\n        steps:\n            - run: echo hello\n")
 
-		var wf map[string]interface{}
+		var wf map[string]any
 		err := yaml.Unmarshal(input, &wf)
 		require.NoError(t, err)
 		assert.Equal(t, wf["name"], "test")
@@ -315,7 +315,7 @@ func Test_yamlV4NodeRoundTrip(t *testing.T) {
 		out, err := yaml.Marshal(wf)
 		require.NoError(t, err)
 
-		var wf2 map[string]interface{}
+		var wf2 map[string]any
 		err = yaml.Unmarshal(out, &wf2)
 		require.NoError(t, err)
 		assert.Equal(t, wf2["name"], "test")

internal/pkg/config/config.go

@@ -5,6 +5,7 @@ package config
 
 import (
 	"fmt"
+	"maps"
 	"os"
 	"path/filepath"
 	"time"
@@ -96,9 +97,7 @@ func LoadDefault(file string) (*Config, error) {
 			if cfg.Runner.Envs == nil {
 				cfg.Runner.Envs = map[string]string{}
 			}
-			for k, v := range envs {
+			maps.Copy(cfg.Runner.Envs, envs)
-				cfg.Runner.Envs[k] = v
-			}
 		}
 	}
 

internal/pkg/report/reporter.go

@@ -5,6 +5,7 @@ package report
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"regexp"
 	"strings"
@@ -37,6 +38,7 @@ type Reporter struct {
 	state   *runnerv1.TaskState
 	stateMu sync.RWMutex
 	outputs sync.Map
+	daemon  chan struct{}
 
 	debugOutputEnabled  bool
 	stopCommandEndToken string
@@ -63,6 +65,7 @@ func NewReporter(ctx context.Context, cancel context.CancelFunc, client client.C
 		state: &runnerv1.TaskState{
 			Id: task.Id,
 		},
+		daemon: make(chan struct{}),
 	}
 
 	if task.Secrets["ACTIONS_STEP_DEBUG"] == "true" {
@@ -75,7 +78,7 @@ func NewReporter(ctx context.Context, cancel context.CancelFunc, client client.C
 func (r *Reporter) ResetSteps(l int) {
 	r.stateMu.Lock()
 	defer r.stateMu.Unlock()
-	for i := 0; i < l; i++ {
+	for i := range l {
 		r.state.Steps = append(r.state.Steps, &runnerv1.StepState{
 			Id: int64(i),
 		})
@@ -93,6 +96,18 @@ func appendIfNotNil[T any](s []*T, v *T) []*T {
 	return s
 }
 
+// isJobStepEntry is used to not report composite step results incorrectly as step result
+// returns true if the logentry is on job level
+// returns false for composite action step messages
+func isJobStepEntry(entry *log.Entry) bool {
+	if v, ok := entry.Data["stepID"]; ok {
+		if v, ok := v.([]string); ok && len(v) > 1 {
+			return false
+		}
+	}
+	return true
+}
+
 func (r *Reporter) Fire(entry *log.Entry) error {
 	r.stateMu.Lock()
 	defer r.stateMu.Unlock()
@@ -109,6 +124,7 @@ func (r *Reporter) Fire(entry *log.Entry) error {
 	if stage != "Main" {
 		if v, ok := entry.Data["jobResult"]; ok {
 			if jobResult, ok := r.parseResult(v); ok {
+				// We need to ensure log upload before this upload
 				r.state.Result = jobResult
 				r.state.StoppedAt = timestamppb.New(timestamp)
 				for _, s := range r.state.Steps {
@@ -162,7 +178,7 @@ func (r *Reporter) Fire(entry *log.Entry) error {
 	} else if !r.duringSteps() {
 		r.logRows = appendIfNotNil(r.logRows, r.parseLogRow(entry))
 	}
-	if v, ok := entry.Data["stepResult"]; ok {
+	if v, ok := entry.Data["stepResult"]; ok && isJobStepEntry(entry) {
 		if stepResult, ok := r.parseResult(v); ok {
 			if step.LogLength == 0 {
 				step.LogIndex = int64(r.logOffset + len(r.logRows))
@@ -176,27 +192,29 @@ func (r *Reporter) Fire(entry *log.Entry) error {
 }
 
 func (r *Reporter) RunDaemon() {
-	if r.closed {
+	r.stateMu.RLock()
-		return
+	closed := r.closed
-	}
+	r.stateMu.RUnlock()
-	if r.ctx.Err() != nil {
+	if closed || r.ctx.Err() != nil {
+		// Acknowledge close
+		close(r.daemon)
 		return
 	}
 
 	_ = r.ReportLog(false)
-	_ = r.ReportState()
+	_ = r.ReportState(false)
 
 	time.AfterFunc(time.Second, r.RunDaemon)
 }
 
-func (r *Reporter) Logf(format string, a ...interface{}) {
+func (r *Reporter) Logf(format string, a ...any) {
 	r.stateMu.Lock()
 	defer r.stateMu.Unlock()
 
 	r.logf(format, a...)
 }
 
-func (r *Reporter) logf(format string, a ...interface{}) {
+func (r *Reporter) logf(format string, a ...any) {
 	if !r.duringSteps() {
 		r.logRows = append(r.logRows, &runnerv1.LogRow{
 			Time:    timestamppb.Now(),
@@ -226,9 +244,8 @@ func (r *Reporter) SetOutputs(outputs map[string]string) {
 }
 
 func (r *Reporter) Close(lastWords string) error {
-	r.closed = true
-
 	r.stateMu.Lock()
+	r.closed = true
 	if r.state.Result == runnerv1.Result_RESULT_UNSPECIFIED {
 		if lastWords == "" {
 			lastWords = "Early termination"
@@ -251,13 +268,23 @@ func (r *Reporter) Close(lastWords string) error {
 		})
 	}
 	r.stateMu.Unlock()
+	// Wait for Acknowledge
+	select {
+	case <-r.daemon:
+	case <-time.After(60 * time.Second):
+		close(r.daemon)
+		log.Error("No Response from RunDaemon for 60s, continue best effort")
+	}
 
-	return retry.Do(func() error {
+	// Report the job outcome even when all log upload retry attempts have been exhausted
-		if err := r.ReportLog(true); err != nil {
+	return errors.Join(
-			return err
+		retry.Do(func() error {
-		}
+			return r.ReportLog(true)
-		return r.ReportState()
+		}, retry.Context(r.ctx)),
-	}, retry.Context(r.ctx))
+		retry.Do(func() error {
+			return r.ReportState(true)
+		}, retry.Context(r.ctx)),
+	)
 }
 
 func (r *Reporter) ReportLog(noMore bool) error {
@@ -280,22 +307,25 @@ func (r *Reporter) ReportLog(noMore bool) error {
 
 	ack := int(resp.Msg.AckIndex)
 	if ack < r.logOffset {
-		return fmt.Errorf("submitted logs are lost")
+		return errors.New("submitted logs are lost")
 	}
 
 	r.stateMu.Lock()
 	r.logRows = r.logRows[ack-r.logOffset:]
+	submitted := r.logOffset + len(rows)
 	r.logOffset = ack
 	r.stateMu.Unlock()
 
-	if noMore && ack < r.logOffset+len(rows) {
+	if noMore && ack < submitted {
-		return fmt.Errorf("not all logs are submitted")
+		return errors.New("not all logs are submitted")
 	}
 
 	return nil
 }
 
-func (r *Reporter) ReportState() error {
+// ReportState only reports the job result if reportResult is true
+// RunDaemon never reports results even if result is set
+func (r *Reporter) ReportState(reportResult bool) error {
 	r.clientM.Lock()
 	defer r.clientM.Unlock()
 
@@ -303,8 +333,13 @@ func (r *Reporter) ReportState() error {
 	state := proto.Clone(r.state).(*runnerv1.TaskState)
 	r.stateMu.RUnlock()
 
+	// Only report result from Close to reliable sent logs
+	if !reportResult {
+		state.Result = runnerv1.Result_RESULT_UNSPECIFIED
+	}
+
 	outputs := make(map[string]string)
-	r.outputs.Range(func(k, v interface{}) bool {
+	r.outputs.Range(func(k, v any) bool {
 		if val, ok := v.(string); ok {
 			outputs[k.(string)] = val
 		}
@@ -328,7 +363,7 @@ func (r *Reporter) ReportState() error {
 	}
 
 	var noSent []string
-	r.outputs.Range(func(k, v interface{}) bool {
+	r.outputs.Range(func(k, v any) bool {
 		if _, ok := v.(string); ok {
 			noSent = append(noSent, k.(string))
 		}
@@ -359,7 +394,7 @@ var stringToResult = map[string]runnerv1.Result{
 	"cancelled": runnerv1.Result_RESULT_CANCELLED,
 }
 
-func (r *Reporter) parseResult(result interface{}) (runnerv1.Result, bool) {
+func (r *Reporter) parseResult(result any) (runnerv1.Result, bool) {
 	str := ""
 	if v, ok := result.(string); ok { // for jobResult
 		str = v
@@ -373,7 +408,7 @@ func (r *Reporter) parseResult(result interface{}) (runnerv1.Result, bool) {
 
 var cmdRegex = regexp.MustCompile(`^::([^ :]+)( .*)?::(.*)$`)
 
-func (r *Reporter) handleCommand(originalContent, command, parameters, value string) *string {
+func (r *Reporter) handleCommand(originalContent, command, value string) *string {
 	if r.stopCommandEndToken != "" && command != r.stopCommandEndToken {
 		return &originalContent
 	}
@@ -419,7 +454,7 @@ func (r *Reporter) parseLogRow(entry *log.Entry) *runnerv1.LogRow {
 
 	matches := cmdRegex.FindStringSubmatch(content)
 	if matches != nil {
-		if output := r.handleCommand(content, matches[1], matches[2], matches[3]); output != nil {
+		if output := r.handleCommand(content, matches[1], matches[3]); output != nil {
 			content = *output
 		} else {
 			return nil

internal/pkg/report/reporter_test.go

@@ -5,8 +5,11 @@ package report
 
 import (
 	"context"
+	"errors"
 	"strings"
+	"sync"
 	"testing"
+	"time"
 
 	runnerv1 "code.gitea.io/actions-proto-go/runner/v1"
 	connect_go "connectrpc.com/connect"
@@ -15,6 +18,7 @@ import (
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 	"google.golang.org/protobuf/types/known/structpb"
+	"google.golang.org/protobuf/types/known/timestamppb"
 
 	"gitea.com/gitea/act_runner/internal/pkg/client/mocks"
 )
@@ -169,29 +173,165 @@ func TestReporter_Fire(t *testing.T) {
 			return connect_go.NewResponse(&runnerv1.UpdateTaskResponse{}), nil
 		})
 		ctx, cancel := context.WithCancel(context.Background())
-		taskCtx, err := structpb.NewStruct(map[string]interface{}{})
+		taskCtx, err := structpb.NewStruct(map[string]any{})
 		require.NoError(t, err)
 		reporter := NewReporter(ctx, cancel, client, &runnerv1.Task{
 			Context: taskCtx,
 		})
+		reporter.RunDaemon()
 		defer func() {
-			assert.NoError(t, reporter.Close(""))
+			require.NoError(t, reporter.Close(""))
 		}()
 		reporter.ResetSteps(5)
 
-		dataStep0 := map[string]interface{}{
+		dataStep0 := map[string]any{
 			"stage":      "Main",
 			"stepNumber": 0,
 			"raw_output": true,
 		}
 
-		assert.NoError(t, reporter.Fire(&log.Entry{Message: "regular log line", Data: dataStep0}))
+		require.NoError(t, reporter.Fire(&log.Entry{Message: "regular log line", Data: dataStep0}))
-		assert.NoError(t, reporter.Fire(&log.Entry{Message: "::debug::debug log line", Data: dataStep0}))
+		require.NoError(t, reporter.Fire(&log.Entry{Message: "::debug::debug log line", Data: dataStep0}))
-		assert.NoError(t, reporter.Fire(&log.Entry{Message: "regular log line", Data: dataStep0}))
+		require.NoError(t, reporter.Fire(&log.Entry{Message: "regular log line", Data: dataStep0}))
-		assert.NoError(t, reporter.Fire(&log.Entry{Message: "::debug::debug log line", Data: dataStep0}))
+		require.NoError(t, reporter.Fire(&log.Entry{Message: "::debug::debug log line", Data: dataStep0}))
-		assert.NoError(t, reporter.Fire(&log.Entry{Message: "::debug::debug log line", Data: dataStep0}))
+		require.NoError(t, reporter.Fire(&log.Entry{Message: "::debug::debug log line", Data: dataStep0}))
-		assert.NoError(t, reporter.Fire(&log.Entry{Message: "regular log line", Data: dataStep0}))
+		require.NoError(t, reporter.Fire(&log.Entry{Message: "regular log line", Data: dataStep0}))
+		require.NoError(t, reporter.Fire(&log.Entry{Message: "composite step result", Data: map[string]any{
+			"stage":      "Main",
+			"stepID":     []string{"0", "0"},
+			"stepNumber": 0,
+			"raw_output": true,
+			"stepResult": "failure",
+		}}))
+		assert.Equal(t, runnerv1.Result_RESULT_UNSPECIFIED, reporter.state.Steps[0].Result)
+		require.NoError(t, reporter.Fire(&log.Entry{Message: "step result", Data: map[string]any{
+			"stage":      "Main",
+			"stepNumber": 0,
+			"raw_output": true,
+			"stepResult": "success",
+		}}))
+		assert.Equal(t, runnerv1.Result_RESULT_SUCCESS, reporter.state.Steps[0].Result)
 
-		assert.Equal(t, int64(3), reporter.state.Steps[0].LogLength)
+		assert.Equal(t, int64(5), reporter.state.Steps[0].LogLength)
 	})
 }
+
+// TestReporter_EphemeralRunnerDeletion reproduces the exact scenario from
+// https://gitea.com/gitea/act_runner/issues/793:
+//
+//  1. RunDaemon calls ReportLog(false) — runner is still alive
+//  2. Close() updates state to Result=FAILURE (between RunDaemon's ReportLog and ReportState)
+//  3. RunDaemon's ReportState() would clone the completed state and send it,
+//     but the fix makes ReportState return early when closed, preventing this
+//  4. Close's ReportLog(true) succeeds because the runner was not deleted
+func TestReporter_EphemeralRunnerDeletion(t *testing.T) {
+	runnerDeleted := false
+
+	client := mocks.NewClient(t)
+	client.On("UpdateLog", mock.Anything, mock.Anything).Return(
+		func(_ context.Context, req *connect_go.Request[runnerv1.UpdateLogRequest]) (*connect_go.Response[runnerv1.UpdateLogResponse], error) {
+			if runnerDeleted {
+				return nil, errors.New("runner has been deleted")
+			}
+			return connect_go.NewResponse(&runnerv1.UpdateLogResponse{
+				AckIndex: req.Msg.Index + int64(len(req.Msg.Rows)),
+			}), nil
+		},
+	)
+	client.On("UpdateTask", mock.Anything, mock.Anything).Maybe().Return(
+		func(_ context.Context, req *connect_go.Request[runnerv1.UpdateTaskRequest]) (*connect_go.Response[runnerv1.UpdateTaskResponse], error) {
+			// Server deletes ephemeral runner when it receives a completed state
+			if req.Msg.State != nil && req.Msg.State.Result != runnerv1.Result_RESULT_UNSPECIFIED {
+				runnerDeleted = true
+			}
+			return connect_go.NewResponse(&runnerv1.UpdateTaskResponse{}), nil
+		},
+	)
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	taskCtx, err := structpb.NewStruct(map[string]any{})
+	require.NoError(t, err)
+	reporter := NewReporter(ctx, cancel, client, &runnerv1.Task{Context: taskCtx})
+	reporter.ResetSteps(1)
+
+	// Fire a log entry to create pending data
+	require.NoError(t, reporter.Fire(&log.Entry{
+		Message: "build output",
+		Data:    log.Fields{"stage": "Main", "stepNumber": 0, "raw_output": true},
+	}))
+
+	// Step 1: RunDaemon calls ReportLog(false) — runner is still alive
+	require.NoError(t, reporter.ReportLog(false))
+
+	// Step 2: Close() updates state — sets Result=FAILURE and marks steps cancelled.
+	// In the real race, this happens while RunDaemon is between ReportLog and ReportState.
+	reporter.stateMu.Lock()
+	reporter.closed = true
+	for _, v := range reporter.state.Steps {
+		if v.Result == runnerv1.Result_RESULT_UNSPECIFIED {
+			v.Result = runnerv1.Result_RESULT_CANCELLED
+		}
+	}
+	reporter.state.Result = runnerv1.Result_RESULT_FAILURE
+	reporter.logRows = append(reporter.logRows, &runnerv1.LogRow{
+		Time:    timestamppb.Now(),
+		Content: "Early termination",
+	})
+	reporter.state.StoppedAt = timestamppb.Now()
+	reporter.stateMu.Unlock()
+
+	// Step 3: RunDaemon's ReportState() — with the fix, this returns early
+	// because closed=true, preventing the server from deleting the runner.
+	require.NoError(t, reporter.ReportState(false))
+	assert.False(t, runnerDeleted, "runner must not be deleted by RunDaemon's ReportState")
+
+	// Step 4: Close's final log upload succeeds because the runner is still alive.
+	// Flush pending rows first, then send the noMore signal (matching Close's retry behavior).
+	require.NoError(t, reporter.ReportLog(false))
+	// Acknowledge Close as done in daemon
+	close(reporter.daemon)
+	err = reporter.ReportLog(true)
+	require.NoError(t, err, "final log upload must not fail: runner should not be deleted before Close finishes sending logs")
+	err = reporter.ReportState(true)
+	require.NoError(t, err, "final state update should work: runner should not be deleted before Close finishes sending logs")
+}
+
+func TestReporter_RunDaemonClose_Race(t *testing.T) {
+	client := mocks.NewClient(t)
+	client.On("UpdateLog", mock.Anything, mock.Anything).Return(
+		func(_ context.Context, req *connect_go.Request[runnerv1.UpdateLogRequest]) (*connect_go.Response[runnerv1.UpdateLogResponse], error) {
+			return connect_go.NewResponse(&runnerv1.UpdateLogResponse{
+				AckIndex: req.Msg.Index + int64(len(req.Msg.Rows)),
+			}), nil
+		},
+	)
+	client.On("UpdateTask", mock.Anything, mock.Anything).Return(
+		func(_ context.Context, req *connect_go.Request[runnerv1.UpdateTaskRequest]) (*connect_go.Response[runnerv1.UpdateTaskResponse], error) {
+			return connect_go.NewResponse(&runnerv1.UpdateTaskResponse{}), nil
+		},
+	)
+
+	ctx, cancel := context.WithCancel(context.Background())
+	taskCtx, err := structpb.NewStruct(map[string]any{})
+	require.NoError(t, err)
+	reporter := NewReporter(ctx, cancel, client, &runnerv1.Task{
+		Context: taskCtx,
+	})
+	reporter.ResetSteps(1)
+
+	// Start the daemon loop in a separate goroutine.
+	// RunDaemon reads r.closed and reschedules itself via time.AfterFunc.
+	var wg sync.WaitGroup
+	wg.Go(func() {
+		reporter.RunDaemon()
+	})
+
+	// Close concurrently — this races with RunDaemon on r.closed.
+	require.NoError(t, reporter.Close(""))
+
+	// Cancel context so pending AfterFunc callbacks exit quickly.
+	cancel()
+	wg.Wait()
+	time.Sleep(2 * time.Second)
+}