fix(k8s): retry the startup exec until the kubelet accepts it (#1337)

A new node can run pods before its kubelet has a serving certificate, so the first exec into the job container fails with `error dialing backend: remote error: tls: internal error` and the job fails during setup.

That exec only reads the arch and PATH, so it now retries for up to a minute unless the API server rejects it (4xx) or the command ran.

Reviewed-on: https://gitea.com/gitea/runner/pulls/1337
Reviewed-by: bircni <[email protected]>
Reviewed-by: Lunny Xiao <[email protected]>
Co-authored-by: Zettat123 <[email protected]>

.gitea/workflows/release-nightly.yml

@@ -11,7 +11,6 @@ on:
 
 env:
   DOCKER_ORG: gitea
-  DOCKER_LATEST: nightly
 
 jobs:
   goreleaser:

.gitea/workflows/release-tag.yml

@@ -69,7 +69,6 @@ jobs:
       image: catthehacker/ubuntu:act-latest
     env:
       DOCKER_ORG: gitea
-      DOCKER_LATEST: latest
     steps:
       - name: Checkout
         uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7

.goreleaser.yaml

@@ -28,38 +28,18 @@ builds:
   ignore:
     - goos: darwin
       goarch: arm
-    - goos: darwin
-      goarch: ppc64le
     - goos: darwin
       goarch: s390x
-    - goos: windows
-      goarch: ppc64le
     - goos: windows
       goarch: s390x
     - goos: windows
       goarch: arm
-      goarm: "5"
-    - goos: windows
-      goarch: arm
-      goarm: "6"
-    - goos: windows
-      goarch: arm
-      goarm: "7"
     - goos: windows
       goarch: arm64
-    - goos: freebsd
-      goarch: ppc64le
     - goos: freebsd
       goarch: s390x
     - goos: freebsd
       goarch: arm
-      goarm: "5"
-    - goos: freebsd
-      goarch: arm
-      goarm: "6"
-    - goos: freebsd
-      goarch: arm
-      goarm: "7"
     - goos: freebsd
       goarch: arm64
   flags:

DEVELOPMENT.md

@@ -28,8 +28,8 @@ depending on the prefix:
 - **No prefix?** Do not escape, and split multi-line values into one row each.
 - **Interpolating a secret?** Masking runs after escaping, so `AppendSecretMasker` registers the
   encoded forms too.
-- Command *properties* also escape `%3A` and `%2C`, which the UI never decodes, so the reporter
-  decodes exactly those two when folding a location into an annotation.
+- Command *properties* also escape `%3A` and `%2C`, which the UI never decodes, so the command
+  handler folds an annotation's location into its decoded message and escapes the result again.
 
 ## Logging
 

Makefile

@@ -14,7 +14,6 @@ GOFILES := $(shell find . -type f -name "*.go" -o -name "go.mod" ! -name "genera
 DOCKER_IMAGE ?= gitea/runner
 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/[email protected] # renovate: datasource=go
 GOVULNCHECK_PACKAGE ?= golang.org/x/vuln/cmd/[email protected] # renovate: datasource=go
@@ -34,15 +33,12 @@ endif
 
 ifeq ($(OS), Windows_NT)
 	GOFLAGS := -v -buildmode=exe
-	EXECUTABLE ?= $(EXECUTABLE).exe
 	GO_ENV_WINDOWS := set GOOS=windows&&
 else ifeq ($(OS), Windows)
 	GOFLAGS := -v -buildmode=exe
-	EXECUTABLE ?= $(EXECUTABLE).exe
 	GO_ENV_WINDOWS := set GOOS=windows&&
 else
 	GOFLAGS := -v
-	EXECUTABLE ?= $(EXECUTABLE)
 	GO_ENV_WINDOWS := GOOS=windows
 endif
 

act/artifactcache/handler_v2_test.go

@@ -19,8 +19,6 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-// v2Call posts a twirp request to the cache service and returns the decoded response.
-// Field names are the proto ones, which is what the toolkit's client sends.
 func v2Call(t *testing.T, handler *Handler, client *http.Client, method string, request any) map[string]any {
 	t.Helper()
 
@@ -37,7 +35,6 @@ func v2Call(t *testing.T, handler *Handler, client *http.Client, method string,
 	return got
 }
 
-// putBlob uploads to a signed URL and returns the status, so a test can assert a refusal.
 func putBlob(t *testing.T, url string, content []byte) int {
 	t.Helper()
 
@@ -47,8 +44,6 @@ func putBlob(t *testing.T, url string, content []byte) int {
 	require.NoError(t, err)
 	defer resp.Body.Close()
 	if resp.StatusCode == http.StatusCreated {
-		// The Azure SDK client dereferences this header without checking, so a blob upload that
-		// omits it panics the caller rather than failing it.
 		require.NotEmpty(t, resp.Header.Get("x-ms-request-id"))
 	}
 	return resp.StatusCode
@@ -67,8 +62,6 @@ func getURL(t *testing.T, url string) []byte {
 	return body
 }
 
-// saveV2 runs the reserve/upload/finalize sequence and returns the finalize response along
-// with the upload URL it used.
 func saveV2(t *testing.T, handler *Handler, key, version string, content []byte) (finalized map[string]any, uploadURL string) {
 	t.Helper()
 
@@ -84,9 +77,6 @@ func saveV2(t *testing.T, handler *Handler, key, version string, content []byte)
 	}), uploadURL
 }
 
-// The whole round trip an actions/cache v2 client makes, plus the guarantees on the signed
-// URLs it is handed: unsigned requests are refused, an upload URL cannot be replayed to read
-// or to replace a finalized entry.
 func TestCacheServiceV2RoundTrip(t *testing.T) {
 	handler := newTestHandler(t, Policy{})
 	content := []byte("the cached archive")
@@ -98,10 +88,8 @@ func TestCacheServiceV2RoundTrip(t *testing.T) {
 	require.Equal(t, true, finalized["ok"])
 	assert.NotEmpty(t, finalized["entry_id"])
 
-	// The upload URL outlives the finalize call, so replaying it must not poison the entry,
-	// and it is an upload URL only: nothing reads a blob back through it.
 	assert.Equal(t, http.StatusBadRequest, putBlob(t, uploadURL, []byte("poisoned")))
-	resp, err := http.Get(uploadURL) //nolint:noctx // the URL is the server under test
+	resp, err := http.Get(uploadURL)
 	require.NoError(t, err)
 	defer resp.Body.Close()
 	assert.Equal(t, http.StatusMethodNotAllowed, resp.StatusCode)
@@ -114,8 +102,6 @@ func TestCacheServiceV2RoundTrip(t *testing.T) {
 	assert.Equal(t, content, getURL(t, downloadURL))
 }
 
-// A large archive is staged as blocks and only put in order by the final block list, so
-// blocks that arrive out of order must still be assembled the way the client asked.
 func TestCacheServiceV2BlockUpload(t *testing.T) {
 	handler := newTestHandler(t, Policy{})
 
@@ -130,7 +116,6 @@ func TestCacheServiceV2BlockUpload(t *testing.T) {
 		blocks[blockID] = []byte(part)
 		order = append(order, blockID)
 	}
-	// Upload in an order that is not the block list order.
 	for _, blockID := range []string{order[2], order[0], order[1]} {
 		require.Equal(t, http.StatusCreated, putBlob(t, uploadURL+"&comp=block&blockid="+blockID, blocks[blockID]))
 	}
@@ -174,8 +159,6 @@ func TestCacheServiceV2Lookups(t *testing.T) {
 		assert.Equal(t, false, got["ok"])
 	})
 
-	// The toolkit serialises with the proto field names; the camelCase spellings of the same
-	// proto JSON mapping are accepted alongside them.
 	for _, field := range []string{"restore_keys", "restoreKeys"} {
 		t.Run("restore keys match by prefix, spelled "+field, func(t *testing.T) {
 			got := v2Call(t, handler, testClient, "GetCacheEntryDownloadURL", map[string]any{
@@ -191,9 +174,6 @@ func TestCacheServiceV2Lookups(t *testing.T) {
 		assert.Equal(t, false, again["ok"])
 	})
 
-	// A key that is only a prefix of an existing one is a different entry, so the
-	// reservation check must be exact and not a restore-key prefix match, or the shorter
-	// key would be reported as existing and silently never saved.
 	t.Run("a prefix of an existing key is still reserved", func(t *testing.T) {
 		reserved := v2Call(t, handler, testClient, "CreateCacheEntry", map[string]any{"key": "deps", "version": "v1"})
 		require.Equal(t, true, reserved["ok"])
@@ -219,7 +199,6 @@ func TestCacheServiceV2Lookups(t *testing.T) {
 		assert.Equal(t, false, got["ok"])
 	})
 
-	// The size the client declares is what Commit validates the assembled archive against.
 	t.Run("finalizing with the wrong size is not ok", func(t *testing.T) {
 		created := v2Call(t, handler, testClient, "CreateCacheEntry", map[string]any{"key": "wrong-size", "version": "v1"})
 		require.Equal(t, http.StatusCreated, putBlob(t, created["signed_upload_url"].(string), []byte("four")))
@@ -230,8 +209,6 @@ func TestCacheServiceV2Lookups(t *testing.T) {
 		assert.Equal(t, false, got["ok"])
 	})
 
-	// Both API versions are served from one store, so an entry written through v2 is a hit for
-	// a v1 client asking for the same key and version.
 	t.Run("a v1 client sees an entry written through v2", func(t *testing.T) {
 		resp, err := testClient.Get(fmt.Sprintf("%s%s/cache?keys=deps-abc&version=v1", handler.ExternalURL(), apiPath))
 		require.NoError(t, err)
@@ -244,7 +221,6 @@ func TestCacheServiceV2Lookups(t *testing.T) {
 		assert.NotEmpty(t, got["archiveLocation"])
 	})
 
-	// The cache of one repository must stay invisible to another, as it does for the v1 API.
 	t.Run("another repository sees nothing", func(t *testing.T) {
 		handler.RegisterJob("other-runtime-token", JobCredential{Repo: "other/repo"})
 		otherClient := &http.Client{Transport: &bearerTransport{token: "other-runtime-token"}}
@@ -321,20 +297,12 @@ func TestCacheServiceV2ConcurrentSaveAndRetries(t *testing.T) {
 		"another job's upload is not this job's to finalize")
 
 	base := handler.ExternalURL() + apiPath
-	reserveV1, err := json.Marshal(&Request{Key: "v1-made", Version: "v1", Size: int64(len(content))})
-	require.NoError(t, err)
-	resp, err := testClient.Post(base+"/caches", "application/json", bytes.NewReader(reserveV1))
-	require.NoError(t, err)
-	var madeByV1 struct {
-		CacheID uint64 `json:"cacheId"`
-	}
-	require.NoError(t, json.UnmarshalRead(resp.Body, &madeByV1))
-	resp.Body.Close()
+	madeByV1 := reserveCache(t, testClient, base, "v1-made", "v1", int64(len(content)))
 	upload, err := http.NewRequestWithContext(t.Context(), http.MethodPatch,
-		fmt.Sprintf("%s/caches/%d", base, madeByV1.CacheID), bytes.NewReader(content))
+		fmt.Sprintf("%s/caches/%d", base, madeByV1), bytes.NewReader(content))
 	require.NoError(t, err)
 	upload.Header.Set("Content-Range", fmt.Sprintf("bytes 0-%d/*", len(content)-1))
-	resp, err = testClient.Do(upload)
+	resp, err := testClient.Do(upload)
 	require.NoError(t, err)
 	resp.Body.Close()
 	require.Equal(t, http.StatusOK, resp.StatusCode)

act/artifactcache/model.go

@@ -11,13 +11,13 @@ type Request struct {
 }
 
 type Cache struct {
-	ID        uint64 `json:"id" boltholdKey:"ID"`
-	Repo      string `json:"repo" boltholdIndex:"Repo"`
-	Key       string `json:"key" boltholdIndex:"Key"`
-	Version   string `json:"version" boltholdIndex:"Version"`
+	ID        uint64 `json:"id"`
+	Repo      string `json:"repo"`
+	Key       string `json:"key"`
+	Version   string `json:"version"`
 	Size      int64  `json:"cacheSize"`
 	Owner     string `json:"owner"` // hashed bearer, so a job's own retry is not another job's reservation
-	Complete  bool   `json:"complete" boltholdIndex:"Complete"`
-	UsedAt    int64  `json:"usedAt" boltholdIndex:"UsedAt"`
-	CreatedAt int64  `json:"createdAt" boltholdIndex:"CreatedAt"`
+	Complete  bool   `json:"complete"`
+	UsedAt    int64  `json:"usedAt"`
+	CreatedAt int64  `json:"createdAt"`
 }

act/artifactcache/results.go

@@ -4,6 +4,7 @@
 package artifactcache
 
 import (
+	"cmp"
 	"crypto/tls"
 	"errors"
 	"fmt"
@@ -74,14 +75,7 @@ func (h *Handler) forwardOrNotFound(w http.ResponseWriter, r *http.Request) {
 
 // A protocol switch loses the bearer or the agent, and an untrusted instance needs skipped verification.
 func (h *Handler) mustProxy(r *http.Request, cred JobCredential, target *url.URL) bool {
-	if cred.InsecureTLS && target.Scheme == "https" {
-		return true
-	}
-	scheme := r.Header.Get("X-Forwarded-Proto")
-	if scheme == "" {
-		scheme = "http"
-	}
-	return scheme != target.Scheme
+	return cred.InsecureTLS && target.Scheme == "https" || cmp.Or(r.Header.Get("X-Forwarded-Proto"), "http") != target.Scheme
 }
 
 func mergeQuery(target, request string) string {

act/artifactcache/s3_storage.go

@@ -24,6 +24,10 @@ import (
 	"strconv"
 	"strings"
 	"time"
+
+	"gitea.com/gitea/runner/act/common"
+
+	"golang.org/x/sync/errgroup"
 )
 
 // S3Options configures the bucket that stores completed cache entries.
@@ -47,7 +51,7 @@ type cacheStorage interface {
 	Remove(uint64) error
 }
 
-const s3PartSize = 5 << 30 // the largest part S3 accepts
+const s3PartSize = 64 << 20
 
 // s3Storage stages uploads on disk and keeps completed entries and their metadata in a bucket.
 type s3Storage struct {
@@ -69,6 +73,7 @@ func newS3Storage(staging *Storage, opts S3Options) (*s3Storage, error) {
 	}
 	transport, _ := http.DefaultTransport.(*http.Transport)
 	transport = transport.Clone()
+	transport.MaxIdleConnsPerHost = 32            // 4 parts each for the 8 jobs a runner commonly runs
 	transport.ResponseHeaderTimeout = time.Minute // a stalled endpoint must not hold the store forever
 	pathStyle := !strings.HasSuffix(endpoint.Hostname(), ".amazonaws.com")
 	if opts.PathStyle != nil {
@@ -209,26 +214,43 @@ func (s *s3Storage) Commit(id uint64, size int64) (int64, error) {
 	return written, nil
 }
 
-// upload always goes in parts, the only way S3 takes an object over 5 GiB.
-func (s *s3Storage) upload(ctx context.Context, key string, file io.ReaderAt, size int64) error {
+func (s *s3Storage) upload(ctx context.Context, key string, file io.ReaderAt, size int64) (uploadErr error) {
+	if size <= s3PartSize {
+		_, err := s.call(ctx, http.MethodPut, key, nil, io.NewSectionReader(file, 0, size), nil)
+		return err
+	}
 	var created struct {
 		UploadID string `xml:"UploadId"`
 	}
 	if _, err := s.call(ctx, http.MethodPost, key, url.Values{"uploads": {""}}, nil, &created); err != nil {
 		return err
 	}
+	defer func() {
+		if uploadErr != nil && created.UploadID != "" {
+			abortCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), time.Minute)
+			defer cancel()
+			_, _ = s.call(abortCtx, http.MethodDelete, key, url.Values{"uploadId": {created.UploadID}}, nil, nil)
+		}
+	}()
 	var completion struct {
 		XMLName xml.Name `xml:"CompleteMultipartUpload"`
 		Parts   []s3Part `xml:"Part"`
 	}
-	for offset := int64(0); offset == 0 || offset < size; offset += s3PartSize {
-		number, length := len(completion.Parts)+1, min(s3PartSize, size-offset)
-		query := url.Values{"partNumber": {strconv.Itoa(number)}, "uploadId": {created.UploadID}}
-		header, err := s.call(ctx, http.MethodPut, key, query, io.NewSectionReader(file, offset, length), nil)
-		if err != nil {
+	partSize := max(int64(s3PartSize), (size-1)/10000+1)
+	completion.Parts = make([]s3Part, (size-1)/partSize+1)
+	workers, workerCtx := errgroup.WithContext(ctx)
+	workers.SetLimit(4)
+	for part := range completion.Parts {
+		workers.Go(func() (err error) {
+			defer common.RecoverPanic(&err)
+			offset := int64(part) * partSize
+			header, err := s.call(workerCtx, http.MethodPut, key, url.Values{"partNumber": {strconv.Itoa(part + 1)}, "uploadId": {created.UploadID}}, io.NewSectionReader(file, offset, min(partSize, size-offset)), nil)
+			completion.Parts[part] = s3Part{part + 1, header.Get("ETag")}
 			return err
+		})
 	}
-		completion.Parts = append(completion.Parts, s3Part{number, header.Get("ETag")})
+	if err := workers.Wait(); err != nil {
+		return err
 	}
 	body, err := xml.Marshal(completion)
 	if err != nil {

act/artifactcache/s3_storage_test.go

@@ -4,21 +4,48 @@
 package artifactcache
 
 import (
+	"encoding/xml"
+	"io"
 	"net/http"
 	"net/http/httptest"
+	"strings"
+	"sync"
+	"sync/atomic"
 	"testing"
 	"time"
 
-	"github.com/johannesboyne/gofakes3"
-	"github.com/johannesboyne/gofakes3/backend/s3mem"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
 func TestHandler_S3SharesEntriesAndDropsExpiredOnes(t *testing.T) {
-	backend := s3mem.New()
-	require.NoError(t, backend.CreateBucket("cache"))
-	server := httptest.NewServer(gofakes3.New(backend).Server())
+	var objects sync.Map
+	var uploads atomic.Int32
+	server := httptest.NewServer(http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) {
+		key := strings.TrimPrefix(request.URL.Path, "/cache/")
+		switch {
+		case request.Method == http.MethodPut:
+			if strings.Contains(key, "/blobs/") {
+				uploads.Add(1)
+			}
+			body, err := io.ReadAll(request.Body)
+			assert.NoError(t, err)
+			objects.Store(key, body)
+		case request.URL.Query().Has("list-type"):
+			_, _ = io.WriteString(writer, "<ListBucketResult>")
+			objects.Range(func(stored, _ any) bool {
+				_, _ = io.WriteString(writer, "<Contents><Key>"+stored.(string)+"</Key></Contents>")
+				return true
+			})
+			_, _ = io.WriteString(writer, "</ListBucketResult>")
+		default:
+			if body, ok := objects.Load(key); ok {
+				_, _ = writer.Write(body.([]byte))
+			} else {
+				writer.WriteHeader(http.StatusNotFound)
+			}
+		}
+	}))
 	t.Cleanup(server.Close)
 	opts := Options{S3: &S3Options{Endpoint: server.URL, Bucket: "cache", Prefix: "runners", AccessKey: "key", SecretKey: "secret"}}
 	saver, restorer := startTestHandler(t, opts), startTestHandler(t, opts)
@@ -26,17 +53,62 @@ func TestHandler_S3SharesEntriesAndDropsExpiredOnes(t *testing.T) {
 
 	finalized, _ := saveV2(t, saver, "deps", "v1", []byte("archive"))
 	require.Equal(t, true, finalized["ok"])
+	assert.EqualValues(t, 1, uploads.Load())
 	require.NoError(t, restorer.importMetadata(t.Context()))
 	found := v2Call(t, restorer, testClient, "GetCacheEntryDownloadURL", entry)
 	require.Equal(t, true, found["ok"])
 	assert.Equal(t, []byte("archive"), getURL(t, found["signed_download_url"].(string)))
 
-	require.NoError(t, backend.ForceDeleteBucket("cache"))
-	require.NoError(t, backend.CreateBucket("cache"))
+	objects.Clear()
 	require.NoError(t, restorer.importMetadata(t.Context()))
 	assert.Equal(t, true, v2Call(t, restorer, testClient, "CreateCacheEntry", entry)["ok"])
 }
 
+type s3TestTransport func(*http.Request) (*http.Response, error)
+
+func (transport s3TestTransport) RoundTrip(req *http.Request) (*http.Response, error) {
+	return transport(req)
+}
+
+func TestS3StorageUploadsLargeObjectsInPartsAndAbortsOnFailure(t *testing.T) {
+	for _, failingPart := range []string{"", "2"} {
+		var parts, aborts atomic.Int32
+		var completion struct {
+			Parts []s3Part `xml:"Part"`
+		}
+		storage := &s3Storage{bucketURL: httptest.NewRequest(http.MethodGet, "http://cache.local/bucket", nil).URL}
+		storage.client = &http.Client{Transport: s3TestTransport(func(req *http.Request) (*http.Response, error) {
+			query, response := req.URL.Query(), &http.Response{StatusCode: http.StatusOK, Header: make(http.Header), Body: http.NoBody}
+			switch {
+			case query.Has("uploads"):
+				response.Body = io.NopCloser(strings.NewReader("<InitiateMultipartUploadResult><UploadId>upload</UploadId></InitiateMultipartUploadResult>"))
+			case req.Method == http.MethodPut:
+				parts.Add(1)
+				response.Header.Set("ETag", "etag-"+query.Get("partNumber"))
+				if query.Get("partNumber") == failingPart {
+					response.StatusCode, response.Body = http.StatusBadRequest, io.NopCloser(strings.NewReader("<Error><Code>InvalidPart</Code></Error>"))
+				}
+			case req.Method == http.MethodPost:
+				require.NoError(t, xml.NewDecoder(req.Body).Decode(&completion))
+				response.Body = io.NopCloser(strings.NewReader("<CompleteMultipartUploadResult/>"))
+			case req.Method == http.MethodDelete:
+				aborts.Add(1)
+			}
+			return response, nil
+		})}
+		err := storage.upload(t.Context(), "blobs/1", strings.NewReader(""), 2*int64(s3PartSize)+1)
+		assert.EqualValues(t, 3, parts.Load())
+		if failingPart == "" {
+			require.NoError(t, err)
+			assert.Equal(t, []s3Part{{1, "etag-1"}, {2, "etag-2"}, {3, "etag-3"}}, completion.Parts)
+			assert.Zero(t, aborts.Load())
+		} else {
+			require.ErrorContains(t, err, "InvalidPart")
+			assert.EqualValues(t, 1, aborts.Load())
+		}
+	}
+}
+
 func TestS3StorageSignsLikeAWS(t *testing.T) {
 	storage := &s3Storage{opts: S3Options{Region: "us-east-1", AccessKey: "AKIAIOSFODNN7EXAMPLE", SecretKey: "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY"}}
 	req := httptest.NewRequest(http.MethodGet, "https://examplebucket.s3.amazonaws.com/?max-keys=2&prefix=J", nil)

act/artifactcache/storage.go

@@ -32,13 +32,11 @@ func NewStorage(rootDir string) (*Storage, error) {
 }
 
 func (s *Storage) Exist(id uint64) (bool, error) {
-	name := s.filename(id)
-	if _, err := os.Stat(name); os.IsNotExist(err) {
+	_, err := os.Stat(s.filename(id))
+	if os.IsNotExist(err) {
 		return false, nil
-	} else if err != nil {
-		return false, err
 	}
-	return true, nil
+	return err == nil, err
 }
 
 func (s *Storage) Write(id uint64, offset int64, reader io.Reader) error {
@@ -92,9 +90,7 @@ func (s *Storage) OrderBlocks(id uint64, blockIDs []string) error {
 }
 
 func (s *Storage) Commit(id uint64, size int64) (int64, error) {
-	defer func() {
-		_ = os.RemoveAll(s.tempDir(id))
-	}()
+	defer func() { _ = os.RemoveAll(s.tempDir(id)) }()
 
 	name := s.filename(id)
 	tempNames, err := s.tempNames(id)
@@ -153,8 +149,7 @@ func assemble(name string, tempNames []string) (int64, error) {
 }
 
 func (s *Storage) Serve(w http.ResponseWriter, r *http.Request, id uint64) {
-	name := s.filename(id)
-	http.ServeFile(w, r, name)
+	http.ServeFile(w, r, s.filename(id))
 }
 
 // Remove deletes an entry's blob and any staged parts. It reports failure so the caller can

act/artifacts/server.go

@@ -79,8 +79,7 @@ func uploads(router *http.ServeMux, baseDir string) {
 			itemPath += gzipExtension
 		}
 
-		safeRunPath := safeResolve(baseDir, req.PathValue("runId"))
-		safePath := safeResolve(safeRunPath, itemPath)
+		safePath := safeResolve(safeResolve(baseDir, req.PathValue("runId")), itemPath)
 
 		if err := os.MkdirAll(filepath.Dir(safePath), os.ModePerm); err != nil {
 			panic(err)
@@ -120,9 +119,7 @@ func downloads(router *http.ServeMux, baseDir string) {
 	router.HandleFunc("GET /_apis/pipelines/workflows/{runId}/artifacts", func(w http.ResponseWriter, req *http.Request) {
 		runID := req.PathValue("runId")
 
-		safePath := safeResolve(baseDir, runID)
-
-		entries, err := os.ReadDir(safePath)
+		entries, err := os.ReadDir(safeResolve(baseDir, runID))
 		if err != nil {
 			panic(err)
 		}
@@ -156,15 +153,11 @@ func downloads(router *http.ServeMux, baseDir string) {
 
 				// if it was upload as gzip
 				rel = strings.TrimSuffix(rel, gzipExtension)
-				path := filepath.Join(itemPath, rel)
-
-				rel = filepath.ToSlash(rel)
-				path = filepath.ToSlash(path)
 
 				files = append(files, ContainerItem{
-					Path:            path,
+					Path:            filepath.ToSlash(filepath.Join(itemPath, rel)),
 					ItemType:        "file",
-					ContentLocation: fmt.Sprintf("http://%s/artifact/%s/%s/%s", req.Host, container, itemPath, rel),
+					ContentLocation: fmt.Sprintf("http://%s/artifact/%s/%s/%s", req.Host, container, itemPath, filepath.ToSlash(rel)),
 				})
 			}
 			return nil

act/common/dryrun.go

@@ -14,14 +14,9 @@ const dryrunContextKeyVal = dryrunContextKey("dryrun")
 
 // Dryrun returns true if the current context is dryrun
 func Dryrun(ctx context.Context) bool {
-	val := ctx.Value(dryrunContextKeyVal)
-	if val != nil {
-		if dryrun, ok := val.(bool); ok {
+	dryrun, _ := ctx.Value(dryrunContextKeyVal).(bool)
 	return dryrun
 }
-	}
-	return false
-}
 
 // WithDryrun adds a value to the context for dryrun
 func WithDryrun(ctx context.Context, dryrun bool) context.Context {

act/common/executor.go

@@ -21,8 +21,7 @@ type Conditional func(ctx context.Context) bool
 // NewInfoExecutor is an executor that logs messages
 func NewInfoExecutor(format string, args ...any) Executor {
 	return func(ctx context.Context) error {
-		logger := Logger(ctx)
-		logger.Infof(format, args...)
+		Logger(ctx).Infof(format, args...)
 		return nil
 	}
 }
@@ -30,8 +29,7 @@ func NewInfoExecutor(format string, args ...any) Executor {
 // NewDebugExecutor is an executor that logs messages
 func NewDebugExecutor(format string, args ...any) Executor {
 	return func(ctx context.Context) error {
-		logger := Logger(ctx)
-		logger.Debugf(format, args...)
+		Logger(ctx).Debugf(format, args...)
 		return nil
 	}
 }
@@ -39,9 +37,7 @@ func NewDebugExecutor(format string, args ...any) Executor {
 // NewPipelineExecutor creates a new executor from a series of other executors
 func NewPipelineExecutor(executors ...Executor) Executor {
 	if len(executors) == 0 {
-		return func(ctx context.Context) error {
-			return nil
-		}
+		return NewErrorExecutor(nil)
 	}
 	var rtn Executor
 	for _, executor := range executors {
@@ -61,6 +57,13 @@ func NewErrorExecutor(err error) Executor {
 	}
 }
 
+func RecoverPanic(err *error) { // defer directly in workers to keep stacks in host logs
+	if recovered := recover(); recovered != nil {
+		logging.Errorf("panic: %v\n%s", recovered, debug.Stack())
+		*err = fmt.Errorf("panic: %v", recovered)
+	}
+}
+
 // NewParallelExecutor creates a new executor from a parallel of other executors
 func NewParallelExecutor(parallel int, executors ...Executor) Executor {
 	if len(executors) == 0 {
@@ -80,8 +83,8 @@ func NewParallelExecutor(parallel int, executors ...Executor) Executor {
 
 		logging.Infof("NewParallelExecutor: Creating %d workers for %d executors", parallel, len(executors))
 
-		for i := 0; i < parallel; i++ {
-			go func(workerID int, work <-chan Executor, errs chan<- error) {
+		for workerID := range parallel {
+			go func() {
 				logging.Debugf("Worker %d started", workerID)
 				taskCount := 0
 				for executor := range work {
@@ -91,17 +94,12 @@ func NewParallelExecutor(parallel int, executors ...Executor) Executor {
 					// goroutine which would leave the runner process hung.
 					// https://gitea.com/gitea/runner/issues/371
 					errs <- func() (err error) {
-						defer func() {
-							if r := recover(); r != nil {
-								logging.Errorf("panic in executor: %v\n%s", r, debug.Stack())
-								err = fmt.Errorf("panic: %v", r)
-							}
-						}()
+						defer RecoverPanic(&err)
 						return executor(ctx)
 					}()
 				}
 				logging.Debugf("Worker %d finished (%d tasks executed)", workerID, taskCount)
-			}(i, work, errs)
+			}()
 		}
 
 		for i := range executors {
@@ -150,12 +148,7 @@ func (e Executor) If(conditional Conditional) Executor {
 
 // IfNot only runs this executor if conditional is true
 func (e Executor) IfNot(conditional Conditional) Executor {
-	return func(ctx context.Context) error {
-		if !conditional(ctx) {
-			return e(ctx)
-		}
-		return nil
-	}
+	return e.If(func(ctx context.Context) bool { return !conditional(ctx) })
 }
 
 // IfBool only runs this executor if conditional is true

act/common/executor_test.go

@@ -7,8 +7,6 @@ package common
 import (
 	"context"
 	"errors"
-	"reflect"
-	"strings"
 	"sync"
 	"sync/atomic"
 	"testing"
@@ -18,42 +16,25 @@ import (
 )
 
 func TestNewWorkflow(t *testing.T) {
-	assert := assert.New(t)
-
 	ctx := t.Context()
+	require.NoError(t, NewPipelineExecutor()(ctx))
+	require.EqualError(t, NewErrorExecutor(errors.New("test error"))(ctx), "test error")
 
-	// empty
-	emptyWorkflow := NewPipelineExecutor()
-	assert.NoError(emptyWorkflow(ctx)) //nolint:testifylint // pre-existing issue from nektos/act
-
-	// error case
-	errorWorkflow := NewErrorExecutor(errors.New("test error"))
-	assert.Error(errorWorkflow(ctx)) //nolint:testifylint // pre-existing issue from nektos/act
-
-	// multiple success case
 	runcount := 0
-	successWorkflow := NewPipelineExecutor(
-		func(ctx context.Context) error {
+	increment := func(_ctx context.Context) error {
 		runcount++
 		return nil
-		},
-		func(ctx context.Context) error {
-			runcount++
-			return nil
-		})
-	assert.NoError(successWorkflow(ctx)) //nolint:testifylint // pre-existing issue from nektos/act
-	assert.Equal(2, runcount)
+	}
+	require.NoError(t, NewPipelineExecutor(increment, increment)(ctx))
+	assert.Equal(t, 2, runcount)
 }
 
-// concurrencyProbe returns an executor recording the peak number of concurrent copies. Copies
-// block until wantActive are in flight so the peak is exact without sleeping, and later copies
-// find the gate already open so the last one still finishes with no partner left.
 func concurrencyProbe(wantActive int32) (exec Executor, count, maxActive *atomic.Int32) {
 	var counted, active, peak atomic.Int32
 	var once sync.Once
 	reached := make(chan struct{})
 
-	return func(ctx context.Context) error {
+	return func(_ctx context.Context) error {
 		counted.Add(1)
 		running := active.Add(1)
 		for {
@@ -72,18 +53,14 @@ func concurrencyProbe(wantActive int32) (exec Executor, count, maxActive *atomic
 }
 
 func TestNewParallelExecutor(t *testing.T) {
-	ctx := t.Context()
-
-	exec, count, maxActive := concurrencyProbe(2)
-	require.NoError(t, NewParallelExecutor(2, exec, exec, exec)(ctx))
-	assert.Equal(t, int32(3), count.Load(), "should run all 3 executors")
-	assert.Equal(t, int32(2), maxActive.Load(), "should run at most 2 executors in parallel")
-
-	// parallelism below 1 falls back to a single worker
-	exec, count, maxActive = concurrencyProbe(1)
-	require.NoError(t, NewParallelExecutor(0, exec, exec, exec)(ctx))
-	assert.Equal(t, int32(3), count.Load(), "should run all 3 executors")
-	assert.Equal(t, int32(1), maxActive.Load(), "should run at most 1 executor in parallel")
+	for name, parallel := range map[string]int{"parallel": 2, "below minimum": 0} {
+		t.Run(name, func(t *testing.T) {
+			exec, count, maxActive := concurrencyProbe(int32(max(1, parallel)))
+			require.NoError(t, NewParallelExecutor(parallel, exec, exec, exec)(t.Context()))
+			assert.Equal(t, int32(3), count.Load())
+			assert.Equal(t, int32(max(1, parallel)), maxActive.Load())
+		})
+	}
 }
 
 func TestNewParallelExecutorEmpty(t *testing.T) {
@@ -99,66 +76,49 @@ func TestNewParallelExecutorEmpty(t *testing.T) {
 	assert.ErrorIs(err, context.Canceled)
 }
 
-func TestNewParallelExecutorFailed(t *testing.T) {
-	assert := assert.New(t)
-
+func TestNewParallelExecutorRunsAllTasksAfterFailureOrCancellation(t *testing.T) {
+	for name, tc := range map[string]struct {
+		canceled        bool
+		parallel, tasks int
+	}{
+		"failure":              {false, 2, 5},
+		"canceled":             {true, 2, 5},
+		"canceled single task": {true, 1, 1},
+	} {
+		t.Run(name, func(t *testing.T) {
 			ctx, cancel := context.WithCancel(t.Context())
+			defer cancel()
+			if tc.canceled {
 				cancel()
-
-	count := 0
-	errorWorkflow := NewPipelineExecutor(func(ctx context.Context) error {
-		count++
-		return errors.New("fake error")
-	})
-	err := NewParallelExecutor(1, errorWorkflow)(ctx)
-	assert.Equal(1, count)
-	assert.ErrorIs(context.Canceled, err)
 			}
-
-func TestNewParallelExecutorCanceled(t *testing.T) {
-	assert := assert.New(t)
-
-	ctx, cancel := context.WithCancel(t.Context())
-	cancel()
-
-	errExpected := errors.New("fake error")
-
-	var count atomic.Int32
-	successWorkflow := NewPipelineExecutor(func(ctx context.Context) error {
-		count.Add(1)
-		return nil
-	})
-	errorWorkflow := NewPipelineExecutor(func(ctx context.Context) error {
-		count.Add(1)
-		return errExpected
-	})
-	err := NewParallelExecutor(3, errorWorkflow, successWorkflow, successWorkflow)(ctx)
-	assert.Equal(int32(3), count.Load())
-	assert.Error(errExpected, err) //nolint:testifylint // pre-existing issue from nektos/act
-}
-
-func TestNewParallelExecutorRunsRemainingAfterFailure(t *testing.T) {
-	var successCount atomic.Int32
-	executors := make([]Executor, 5)
+			var calls atomic.Int32
+			executors := make([]Executor, tc.tasks)
 			for i := range executors {
-		executors[i] = func(ctx context.Context) error {
-			if i == 2 {
+				executors[i] = func(_ctx context.Context) error {
+					calls.Add(1)
+					if i == tc.tasks/2 {
 						return errors.New("fake error")
 					}
-			successCount.Add(1)
 					return nil
 				}
 			}
 
-	require.Error(t, NewParallelExecutor(2, executors...)(t.Context()))
-	assert.Equal(t, int32(4), successCount.Load(), "a failing executor must not stop the others")
+			err := NewParallelExecutor(tc.parallel, executors...)(ctx)
+			if tc.canceled {
+				require.ErrorIs(t, err, context.Canceled)
+			} else {
+				require.EqualError(t, err, "fake error")
+			}
+			assert.Equal(t, int32(tc.tasks), calls.Load())
+		})
+	}
 }
 
 func TestExecutorConditionalsAndFinally(t *testing.T) {
 	ctx := t.Context()
 	var calls []string
 	record := func(name string) Executor {
-		return func(ctx context.Context) error {
+		return func(_ctx context.Context) error {
 			calls = append(calls, name)
 			return nil
 		}
@@ -170,10 +130,7 @@ func TestExecutorConditionalsAndFinally(t *testing.T) {
 	require.NoError(t, record("if-bool").IfBool(true)(ctx))
 	require.NoError(t, record("main").Finally(record("finally"))(ctx))
 
-	want := []string{"if-true", "if-not", "if-bool", "main", "finally"}
-	if !reflect.DeepEqual(calls, want) {
-		t.Fatalf("calls = %v, want %v", calls, want)
-	}
+	assert.Equal(t, []string{"if-true", "if-not", "if-bool", "main", "finally"}, calls)
 }
 
 func TestExecutorFinallyReturnsFinallyErrorWithOriginal(t *testing.T) {
@@ -181,8 +138,5 @@ func TestExecutorFinallyReturnsFinallyErrorWithOriginal(t *testing.T) {
 	finalErr := errors.New("cleanup failed")
 
 	err := NewErrorExecutor(mainErr).Finally(NewErrorExecutor(finalErr))(t.Context())
-	require.Error(t, err)
-	if !strings.Contains(err.Error(), "cleanup failed") || !strings.Contains(err.Error(), "main failed") {
-		t.Fatalf("finally error = %q, want both cleanup and original error", err)
-	}
+	require.EqualError(t, err, "error occurred running finally: cleanup failed (original error: main failed)")
 }

act/common/git/fetch_action.go

@@ -16,14 +16,25 @@ import (
 	"net/url"
 	"os"
 	"path/filepath"
+	"reflect"
 	"runtime"
 	"strconv"
 	"strings"
+	"sync"
 	"time"
 
 	"gitea.com/gitea/runner/act/common"
 )
 
+var actionFetches sync.Map
+
+type actionFetch struct {
+	done     chan struct{}
+	dir, sha string
+	err      error
+	retry    bool // the leader was cancelled or panicked, so followers fetch themselves
+}
+
 var archiveRetryDelay = func(retryAfter string) time.Duration { // as actions/runner backs off
 	if seconds, err := strconv.Atoi(retryAfter); err == nil {
 		return min(max(time.Duration(seconds)*time.Second, 10*time.Second), 10*time.Minute)
@@ -32,6 +43,36 @@ var archiveRetryDelay = func(retryAfter string) time.Duration { // as actions/ru
 }
 
 func FetchAction(ctx context.Context, input NewGitCloneExecutorInput) (string, string, error) { // input.Dir is one clone URL and ref's, holding a directory per commit
+	if !reflect.ValueOf(input).Comparable() {
+		return fetchAction(ctx, input)
+	}
+	for ctx.Err() == nil {
+		call := &actionFetch{done: make(chan struct{}), retry: true}
+		previous, loaded := actionFetches.LoadOrStore(input, call)
+		if !loaded {
+			func() {
+				defer func() { actionFetches.Delete(input); close(call.done) }()
+				call.dir, call.sha, call.err = fetchAction(ctx, input)
+				call.retry = ctx.Err() != nil
+			}()
+			if call.retry {
+				continue
+			}
+			return call.dir, call.sha, call.err
+		}
+		call, _ = previous.(*actionFetch)
+		select {
+		case <-ctx.Done():
+		case <-call.done:
+			if !call.retry && ctx.Err() == nil {
+				return call.dir, call.sha, call.err
+			}
+		}
+	}
+	return "", "", ctx.Err()
+}
+
+func fetchAction(ctx context.Context, input NewGitCloneExecutorInput) (string, string, error) {
 	unlock, err := AcquireCloneLock(ctx, input.Dir)
 	if err != nil {
 		return "", "", err

act/common/git/fetch_action_test.go

@@ -6,6 +6,7 @@ package git
 import (
 	"archive/tar"
 	"bytes"
+	"context"
 	"encoding/base64"
 	"fmt"
 	"net/http"
@@ -16,9 +17,12 @@ import (
 	"path/filepath"
 	"strings"
 	"sync"
+	"sync/atomic"
 	"testing"
 	"time"
 
+	"gitea.com/gitea/runner/act/common"
+
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -102,6 +106,75 @@ func TestFetchAction(t *testing.T) {
 		return NewGitCloneExecutorInput{URL: server.URL + "/org/action", Ref: ref, Dir: t.TempDir(), Token: "secret"}
 	}
 
+	t.Run("shares identical fetches and isolates cancellation, panics and differing inputs", func(t *testing.T) {
+		expectedSha := gitRevParse(t, remote, "main")
+		for _, scenario := range []string{"shared", "leader cancelled", "leader panics", "different token"} {
+			t.Run(scenario, func(t *testing.T) {
+				serve("prefixed", 0)
+				input := newInput("main")
+				started, release := make(chan struct{}), make(chan struct{})
+				releaseFetches := sync.OnceFunc(func() { close(release) })
+				t.Cleanup(releaseFetches)
+				var once sync.Once
+				var panicked atomic.Bool
+				transport := actionFetchTransport(func(request *http.Request) (*http.Response, error) {
+					once.Do(func() { close(started) })
+					select {
+					case <-request.Context().Done():
+						return nil, request.Context().Err()
+					case <-release:
+					}
+					if scenario == "leader panics" && panicked.CompareAndSwap(false, true) {
+						panic("broken archive")
+					}
+					return http.DefaultTransport.RoundTrip(request)
+				})
+				input.Transport = &transport
+				leaderCtx, cancelLeader := context.WithCancel(t.Context())
+				defer cancelLeader()
+				waiting := &actionFetchContext{Context: t.Context(), waiting: make(chan struct{})}
+				followerInput := input
+				expectedRequests := 1
+				if scenario == "different token" {
+					followerInput.Token, expectedRequests = "other", 2
+				}
+				leaderDone, followerDone := make(chan error, 1), make(chan error, 1)
+				fetch := func(ctx context.Context, input NewGitCloneExecutorInput, done chan<- error) {
+					var err error
+					defer func() { done <- err }()
+					defer common.RecoverPanic(&err)
+					var tree, sha string
+					if tree, sha, err = FetchAction(ctx, input); err == nil {
+						assert.Equal(t, expectedSha, sha)
+						assert.Equal(t, filepath.Join(input.Dir, sha), tree)
+					}
+				}
+				go fetch(leaderCtx, input, leaderDone)
+				<-started
+				go fetch(waiting, followerInput, followerDone)
+				<-waiting.waiting
+				if scenario == "leader cancelled" {
+					cancelLeader()
+					require.ErrorIs(t, <-leaderDone, context.Canceled)
+				}
+				releaseFetches()
+				switch scenario {
+				case "leader cancelled":
+				case "leader panics":
+					require.ErrorContains(t, <-leaderDone, "panic: broken archive")
+				default:
+					require.NoError(t, <-leaderDone)
+				}
+				require.NoError(t, <-followerDone)
+				_, headers := served()
+				assert.Len(t, headers, expectedRequests)
+				if scenario == "different token" {
+					assert.NotEqual(t, headers[0].Get("Authorization"), headers[1].Get("Authorization"))
+				}
+			})
+		}
+	})
+
 	t.Run("downloads, revalidates and follows a moved tag", func(t *testing.T) {
 		serve("prefixed", 0)
 		input := newInput("v1")
@@ -200,6 +273,23 @@ func TestFetchAction(t *testing.T) {
 	})
 }
 
+type actionFetchTransport func(*http.Request) (*http.Response, error)
+
+func (transport actionFetchTransport) RoundTrip(request *http.Request) (*http.Response, error) {
+	return transport(request)
+}
+
+type actionFetchContext struct {
+	context.Context
+	waiting chan struct{}
+	once    sync.Once
+}
+
+func (ctx *actionFetchContext) Done() <-chan struct{} {
+	ctx.once.Do(func() { close(ctx.waiting) })
+	return ctx.Context.Done()
+}
+
 func TestExtractTreeConfinesEntries(t *testing.T) {
 	outside := t.TempDir()
 	for name, entries := range map[string][]*tar.Header{

act/common/git/git.go

@@ -79,25 +79,25 @@ func (e *Error) Unwrap() error  { return e.err }
 func (e *Error) Commit() string { return e.commit }
 
 // FindGitRevision get the current git revision
-func FindGitRevision(ctx context.Context, file string) (shortSHA, sha string, err error) {
+func FindGitRevision(ctx context.Context, file string) (string, error) {
 	logger := common.Logger(ctx)
-	sha, err = gitOutput(ctx, file, "rev-parse", "--verify", "HEAD^{commit}")
+	sha, err := gitOutput(ctx, file, "rev-parse", "--verify", "HEAD^{commit}")
 	if err != nil {
 		logger.WithError(err).Error("path", file, "not located inside a git repository")
-		return "", "", err
+		return "", err
 	}
 	if len(sha) < 7 {
-		return "", "", errors.New("HEAD could not be resolved")
+		return "", errors.New("HEAD could not be resolved")
 	}
 	logger.Debugf("Found revision: %s", sha)
-	return sha[:7], sha, nil
+	return sha, nil
 }
 
 // FindGitRef get the current git ref
 func FindGitRef(ctx context.Context, file string) (string, error) {
 	logger := common.Logger(ctx)
 	logger.Debugf("Loading revision from git directory")
-	_, revision, err := FindGitRevision(ctx, file)
+	revision, err := FindGitRevision(ctx, file)
 	if err != nil {
 		return "", err
 	}

act/common/git/git_test.go

@@ -67,42 +67,20 @@ func TestErrorWrapsCommitAndCause(t *testing.T) {
 	require.Equal(t, "abc123", err.Commit())
 }
 
-func cleanGitHooks(dir string) error {
-	hooksDir := filepath.Join(dir, ".git", "hooks")
-	files, err := os.ReadDir(hooksDir)
-	if err != nil {
-		if os.IsNotExist(err) {
-			return nil
-		}
-		return err
-	}
-	for _, f := range files {
-		if f.IsDir() {
-			continue
-		}
-		relName := filepath.Join(hooksDir, f.Name())
-		if err := os.Remove(relName); err != nil {
-			return err
-		}
-	}
-	return nil
-}
-
 func TestFindGitMetadataOfSHA256RepositoryUsesOrigin(t *testing.T) {
 	t.Parallel()
 	basedir := t.TempDir()
 	const remoteURL = "https://github.com/owner/repo.git"
 	require.NoError(t, gitCmd("init", "--object-format=sha256", "--initial-branch=main", basedir))
-	require.NoError(t, cleanGitHooks(basedir))
+	require.NoError(t, os.RemoveAll(filepath.Join(basedir, ".git", "hooks")))
 	require.NoError(t, gitCmd("-C", basedir, "commit", "--allow-empty", "-m", "init"))
 	require.NoError(t, gitCmd("-C", basedir, "remote", "add", "origin", remoteURL))
 	require.NoError(t, gitCmd("-C", basedir, "config", "--add", "remote.origin.url", "https://github.com/other/repo.git"))
 
-	shortSHA, sha, err := FindGitRevision(t.Context(), basedir)
+	sha, err := FindGitRevision(t.Context(), basedir)
 	require.NoError(t, err)
 	require.Equal(t, gitRevParse(t, basedir, "HEAD"), sha)
 	require.Len(t, sha, 64)
-	require.Equal(t, sha[:7], shortSHA)
 
 	ref, err := FindGitRef(t.Context(), basedir)
 	require.NoError(t, err)
@@ -122,89 +100,66 @@ func TestGitFindRef(t *testing.T) {
 	basedir := t.TempDir()
 
 	for name, tt := range map[string]struct {
-		Prepare func(t *testing.T, dir string)
-		Assert  func(t *testing.T, ref string, err error)
+		commands [][]string
+		ref      string
 	}{
-		"new_repo": {
-			Prepare: func(t *testing.T, dir string) {},
-			Assert: func(t *testing.T, ref string, err error) {
-				require.Error(t, err)
-			},
-		},
+		"new_repo": {},
 		"new_repo_with_commit": {
-			Prepare: func(t *testing.T, dir string) {
-				require.NoError(t, gitCmd("-C", dir, "commit", "--allow-empty", "-m", "msg"))
-			},
-			Assert: func(t *testing.T, ref string, err error) {
-				require.NoError(t, err)
-				require.Equal(t, "refs/heads/master", ref)
-			},
+			commands: [][]string{{"commit", "--allow-empty", "-m", "msg"}},
+			ref:      "refs/heads/master",
 		},
 		"current_head_is_tag": {
-			Prepare: func(t *testing.T, dir string) {
-				require.NoError(t, gitCmd("-C", dir, "commit", "--allow-empty", "-m", "commit msg"))
-				require.NoError(t, gitCmd("-C", dir, "tag", "v1.2.3"))
-				require.NoError(t, gitCmd("-C", dir, "checkout", "v1.2.3"))
-			},
-			Assert: func(t *testing.T, ref string, err error) {
-				require.NoError(t, err)
-				require.Equal(t, "refs/tags/v1.2.3", ref)
+			commands: [][]string{
+				{"commit", "--allow-empty", "-m", "commit msg"},
+				{"tag", "v1.2.3"},
+				{"checkout", "v1.2.3"},
 			},
+			ref: "refs/tags/v1.2.3",
 		},
 		"current_head_is_same_as_tag": {
-			Prepare: func(t *testing.T, dir string) {
-				require.NoError(t, gitCmd("-C", dir, "commit", "--allow-empty", "-m", "1.4.2 release"))
-				require.NoError(t, gitCmd("-C", dir, "tag", "v1.4.2"))
-			},
-			Assert: func(t *testing.T, ref string, err error) {
-				require.NoError(t, err)
-				require.Equal(t, "refs/tags/v1.4.2", ref)
+			commands: [][]string{
+				{"commit", "--allow-empty", "-m", "1.4.2 release"},
+				{"tag", "v1.4.2"},
 			},
+			ref: "refs/tags/v1.4.2",
 		},
 		"current_head_is_not_tag": {
-			Prepare: func(t *testing.T, dir string) {
-				require.NoError(t, gitCmd("-C", dir, "commit", "--allow-empty", "-m", "msg"))
-				require.NoError(t, gitCmd("-C", dir, "tag", "v1.4.2"))
-				require.NoError(t, gitCmd("-C", dir, "commit", "--allow-empty", "-m", "msg2"))
-			},
-			Assert: func(t *testing.T, ref string, err error) {
-				require.NoError(t, err)
-				require.Equal(t, "refs/heads/master", ref)
+			commands: [][]string{
+				{"commit", "--allow-empty", "-m", "msg"},
+				{"tag", "v1.4.2"},
+				{"commit", "--allow-empty", "-m", "msg2"},
 			},
+			ref: "refs/heads/master",
 		},
 		"current_head_is_another_branch": {
-			Prepare: func(t *testing.T, dir string) {
-				require.NoError(t, gitCmd("-C", dir, "checkout", "-b", "mybranch"))
-				require.NoError(t, gitCmd("-C", dir, "commit", "--allow-empty", "-m", "msg"))
-			},
-			Assert: func(t *testing.T, ref string, err error) {
-				require.NoError(t, err)
-				require.Equal(t, "refs/heads/mybranch", ref)
+			commands: [][]string{
+				{"checkout", "-b", "mybranch"},
+				{"commit", "--allow-empty", "-m", "msg"},
 			},
+			ref: "refs/heads/mybranch",
 		},
 	} {
 		t.Run(name, func(t *testing.T) {
 			dir := filepath.Join(basedir, name)
 			require.NoError(t, os.MkdirAll(dir, 0o755))
 			require.NoError(t, gitCmd("-C", dir, "init", "--initial-branch=master"))
-			require.NoError(t, cleanGitHooks(dir))
-			tt.Prepare(t, dir)
+			require.NoError(t, os.RemoveAll(filepath.Join(dir, ".git", "hooks")))
+			for _, command := range tt.commands {
+				require.NoError(t, gitCmd(append([]string{"-C", dir}, command...)...))
+			}
 			ref, err := FindGitRef(t.Context(), dir)
-			tt.Assert(t, ref, err)
+			if tt.ref == "" {
+				require.Error(t, err)
+			} else {
+				require.NoError(t, err)
+				require.Equal(t, tt.ref, ref)
+			}
 		})
 	}
 }
 
 func TestGitCloneExecutor(t *testing.T) {
-	// Build a local bare "remote" so this runs offline and fast. The cases below mirror
-	// the tag/branch/sha/short-sha ref paths the executor handles, formerly exercised by
-	// cloning actions/checkout and anchore/scan-action over the network.
-	remoteDir := t.TempDir()
-	require.NoError(t, gitCmd("init", "--bare", "--initial-branch=main", remoteDir))
-
-	workDir := t.TempDir()
-	require.NoError(t, gitCmd("clone", remoteDir, workDir))
-	require.NoError(t, gitCmd("-C", workDir, "checkout", "-b", "main"))
+	remoteDir, workDir := newGitRemote(t)
 	require.NoError(t, os.WriteFile(filepath.Join(workDir, "action.sh"), []byte("#!/bin/sh\necho hi\n"), 0o755))
 	require.NoError(t, gitCmd("-C", workDir, "add", "action.sh"))
 	require.NoError(t, gitCmd("-C", workDir, "commit", "-m", "initial"))
@@ -212,14 +167,11 @@ func TestGitCloneExecutor(t *testing.T) {
 	require.NoError(t, gitCmd("-C", workDir, "push", "-u", "origin", "main"))
 	require.NoError(t, gitCmd("-C", workDir, "push", "origin", "v2"))
 
-	// A branch with a dash in the name (mirrors the historical scan-action@act-fails case).
 	require.NoError(t, gitCmd("-C", workDir, "checkout", "-b", "act-fails"))
 	require.NoError(t, gitCmd("-C", workDir, "commit", "--allow-empty", "-m", "branch-commit"))
 	require.NoError(t, gitCmd("-C", workDir, "push", "origin", "act-fails"))
 
-	out, err := exec.Command("git", "-C", workDir, "rev-parse", "main").Output()
-	require.NoError(t, err)
-	fullSha := strings.TrimSpace(string(out))
+	fullSha := gitRevParse(t, workDir, "main")
 
 	require.NoError(t, gitCmd("-C", workDir, "push", "origin", "main:refs/heads/"+fullSha[:4]))
 
@@ -265,12 +217,12 @@ func TestGitCloneExecutor(t *testing.T) {
 
 			err := clone(t.Context())
 			if tt.Err != nil {
-				assert.Error(t, err) //nolint:testifylint // pre-existing issue from nektos/act
+				require.Error(t, err)
 				assert.Equal(t, tt.Err, err)
 				assert.Equal(t, tt.Err, clone(t.Context()), "a retry on the cache keeps the error")
 				return
 			}
-			assert.Empty(t, err) //nolint:testifylint // pre-existing issue from nektos/act
+			require.NoError(t, err)
 			marker := filepath.Join(dir, "marker")
 			require.NoError(t, os.WriteFile(marker, nil, 0o644))
 			require.NoError(t, clone(t.Context()))
@@ -285,13 +237,7 @@ func TestGitCloneExecutor(t *testing.T) {
 func TestGitCloneExecutorReclonesWhenOriginURLChanges(t *testing.T) {
 	t.Parallel()
 	createRemote := func(message string) string {
-		remoteDir := t.TempDir()
-		require.NoError(t, gitCmd("init", "--bare", "--initial-branch=main", remoteDir))
-
-		workDir := t.TempDir()
-		require.NoError(t, gitCmd("clone", remoteDir, workDir))
-		require.NoError(t, gitCmd("-C", workDir, "checkout", "-b", "main"))
-		require.NoError(t, gitCmd("-C", workDir, "commit", "--allow-empty", "-m", message))
+		remoteDir, workDir := newGitRemote(t, message)
 		require.NoError(t, gitCmd("-C", workDir, "push", "-u", "origin", "main"))
 
 		return remoteDir
@@ -341,13 +287,7 @@ func TestGitCloneExecutorReclonesWhenOriginURLChanges(t *testing.T) {
 
 func TestGitCloneExecutorFollowsRemoteRefChanges(t *testing.T) {
 	t.Parallel()
-	remoteDir := t.TempDir()
-	require.NoError(t, gitCmd("init", "--bare", "--initial-branch=main", remoteDir))
-	workDir := t.TempDir()
-	require.NoError(t, gitCmd("clone", remoteDir, workDir))
-	require.NoError(t, gitCmd("-C", workDir, "checkout", "-b", "main"))
-	require.NoError(t, gitCmd("-C", workDir, "commit", "--allow-empty", "-m", "initial"))
-	require.NoError(t, gitCmd("-C", workDir, "commit", "--allow-empty", "-m", "feature-1"))
+	remoteDir, workDir := newGitRemote(t, "initial", "feature-1")
 	require.NoError(t, gitCmd("-C", workDir, "push", "-u", "origin", "main"))
 
 	dir := t.TempDir()
@@ -380,16 +320,9 @@ func TestGitCloneExecutorFollowsRemoteRefChanges(t *testing.T) {
 
 func TestGitCloneExecutorOfflineMode(t *testing.T) {
 	t.Parallel()
-	// Build a local "remote" with a single commit on main.
-	remoteDir := t.TempDir()
-	require.NoError(t, gitCmd("init", "--bare", "--initial-branch=main", remoteDir))
-	workDir := t.TempDir()
-	require.NoError(t, gitCmd("clone", remoteDir, workDir))
-	require.NoError(t, gitCmd("-C", workDir, "checkout", "-b", "main"))
-	require.NoError(t, gitCmd("-C", workDir, "commit", "--allow-empty", "-m", "initial"))
+	remoteDir, workDir := newGitRemote(t, "initial")
 	require.NoError(t, gitCmd("-C", workDir, "push", "-u", "origin", "main"))
 
-	// Prime the cache with an online clone of main.
 	cacheDir := t.TempDir()
 	require.NoError(t, NewGitCloneExecutor(NewGitCloneExecutorInput{
 		URL: remoteDir,
@@ -398,8 +331,6 @@ func TestGitCloneExecutorOfflineMode(t *testing.T) {
 	})(t.Context()))
 
 	t.Run("cached branch resolves without fetching", func(t *testing.T) {
-		// Offline reuse of a cached branch must succeed even though ResolveRevision(input.Ref)
-		// finds no local refs/heads/<ref>.
 		err := NewGitCloneExecutor(NewGitCloneExecutorInput{
 			URL:         remoteDir,
 			Ref:         "main",
@@ -414,7 +345,6 @@ func TestGitCloneExecutorOfflineMode(t *testing.T) {
 	})
 
 	t.Run("unresolvable cached ref returns error", func(t *testing.T) {
-		// The ref was never cached; offline mode cannot resolve it and must return an error.
 		err := NewGitCloneExecutor(NewGitCloneExecutorInput{
 			URL:         remoteDir,
 			Ref:         "never-fetched",
@@ -427,15 +357,7 @@ func TestGitCloneExecutorOfflineMode(t *testing.T) {
 
 func TestGitCloneExecutorShallow(t *testing.T) {
 	t.Parallel()
-	// Build a local "remote" with several commits on main plus a tag, so a full clone would pull noticeably more history than a shallow one.
-	remoteDir := t.TempDir()
-	require.NoError(t, gitCmd("init", "--bare", "--initial-branch=main", remoteDir))
-	workDir := t.TempDir()
-	require.NoError(t, gitCmd("clone", remoteDir, workDir))
-	require.NoError(t, gitCmd("-C", workDir, "checkout", "-b", "main"))
-	for _, m := range []string{"c1", "c2", "c3"} {
-		require.NoError(t, gitCmd("-C", workDir, "commit", "--allow-empty", "-m", m))
-	}
+	remoteDir, workDir := newGitRemote(t, "c1", "c2", "c3")
 	require.NoError(t, gitCmd("-C", workDir, "tag", "v1"))
 	unadvertisedSHA := gitRevParse(t, workDir, "HEAD~1")
 	require.NoError(t, gitCmd("-C", workDir, "push", "-u", "origin", "main"))
@@ -443,25 +365,17 @@ func TestGitCloneExecutorShallow(t *testing.T) {
 
 	shallowMarker := func(dir string) string { return filepath.Join(dir, ".git", "shallow") }
 
-	t.Run("branch is cloned shallowly", func(t *testing.T) {
+	for name, ref := range map[string]string{"branch is cloned shallowly": "main", "tag is cloned shallowly": "v1"} {
+		t.Run(name, func(t *testing.T) {
 			dir := t.TempDir()
 			require.NoError(t, NewGitCloneExecutor(NewGitCloneExecutorInput{
-			URL: remoteDir, Ref: "main", Dir: dir, Depth: 1,
+				URL: remoteDir, Ref: ref, Dir: dir, Depth: 1,
 			})(t.Context()))
 			assert.FileExists(t, shallowMarker(dir), "clone should be shallow")
 			assert.Equal(t, 1, gitRevCount(t, dir), "only the tip commit should be present")
 			assert.Equal(t, "c3", gitHeadSubject(t, dir))
 		})
-
-	t.Run("tag is cloned shallowly", func(t *testing.T) {
-		dir := t.TempDir()
-		require.NoError(t, NewGitCloneExecutor(NewGitCloneExecutorInput{
-			URL: remoteDir, Ref: "v1", Dir: dir, Depth: 1,
-		})(t.Context()))
-		assert.FileExists(t, shallowMarker(dir), "clone should be shallow")
-		assert.Equal(t, 1, gitRevCount(t, dir))
-		assert.Equal(t, "c3", gitHeadSubject(t, dir))
-	})
+	}
 
 	unadvertisedRemote := func(t *testing.T, allowUnadvertised bool) string {
 		remote := filepath.Join(t.TempDir(), "remote.git")
@@ -541,12 +455,7 @@ func TestGitCloneExecutorShallow(t *testing.T) {
 
 func TestGitCloneExecutorPinnedHashIgnoresShadowingRef(t *testing.T) {
 	t.Parallel()
-	remoteDir := t.TempDir()
-	require.NoError(t, gitCmd("init", "--bare", "--initial-branch=main", remoteDir))
-	workDir := t.TempDir()
-	require.NoError(t, gitCmd("clone", remoteDir, workDir))
-	require.NoError(t, gitCmd("-C", workDir, "checkout", "-b", "main"))
-	require.NoError(t, gitCmd("-C", workDir, "commit", "--allow-empty", "-m", "pinned"))
+	remoteDir, workDir := newGitRemote(t, "pinned")
 	pinned := gitRevParse(t, workDir, "HEAD")
 	require.NoError(t, gitCmd("-C", workDir, "commit", "--allow-empty", "-m", "decoy"))
 	require.NoError(t, gitCmd("-C", workDir, "tag", pinned))
@@ -564,6 +473,18 @@ func TestGitCloneExecutorPinnedHashIgnoresShadowingRef(t *testing.T) {
 	}
 }
 
+func newGitRemote(t *testing.T, messages ...string) (string, string) {
+	t.Helper()
+	remoteDir, workDir := t.TempDir(), t.TempDir()
+	require.NoError(t, gitCmd("init", "--bare", "--initial-branch=main", remoteDir))
+	require.NoError(t, gitCmd("clone", remoteDir, workDir))
+	require.NoError(t, gitCmd("-C", workDir, "checkout", "-b", "main"))
+	for _, message := range messages {
+		require.NoError(t, gitCmd("-C", workDir, "commit", "--allow-empty", "-m", message))
+	}
+	return remoteDir, workDir
+}
+
 func gitRevParse(t *testing.T, dir, rev string) string {
 	t.Helper()
 	out, err := exec.Command("git", "-C", dir, "rev-parse", rev).Output()
@@ -591,9 +512,6 @@ func gitCmd(args ...string) error {
 	cmd := exec.Command("git", args...)
 	cmd.Stdout = os.Stdout
 	cmd.Stderr = os.Stderr
-	// Inject a deterministic identity and ignore the host's global/system config so commits
-	// succeed regardless of the host having no user.name/user.email (e.g. CI, GITHUB_ACTIONS
-	// unset) or a global commit.gpgsign, and without mutating the developer's ~/.gitconfig.
 	cmd.Env = append(os.Environ(),
 		"GIT_AUTHOR_NAME=Unit Test",
 		"[email protected]",
@@ -695,7 +613,6 @@ func TestAcquireCloneLock(t *testing.T) {
 	})
 }
 
-// An unresponsive remote must not pin a job: the refresh has to be interruptible.
 func TestNewGitCloneExecutorFetchSendsTokenAndHonoursContext(t *testing.T) {
 	t.Parallel()
 	block := make(chan struct{})

act/common/job_error.go

@@ -14,14 +14,9 @@ const jobErrorContextKeyVal = jobErrorContextKey("job.error")
 
 // JobError returns the job error for current context if any
 func JobError(ctx context.Context) error {
-	val := ctx.Value(jobErrorContextKeyVal)
-	if val != nil {
-		if container, ok := val.(map[string]error); ok {
+	container, _ := ctx.Value(jobErrorContextKeyVal).(map[string]error)
 	return container["error"]
 }
-	}
-	return nil
-}
 
 func SetJobError(ctx context.Context, err error) {
 	if container, ok := ctx.Value(jobErrorContextKeyVal).(map[string]error); ok {
@@ -31,6 +26,5 @@ func SetJobError(ctx context.Context, err error) {
 
 // WithJobErrorContainer adds a value to the context as a container for an error
 func WithJobErrorContainer(ctx context.Context) context.Context {
-	container := map[string]error{}
-	return context.WithValue(ctx, jobErrorContextKeyVal, container)
+	return context.WithValue(ctx, jobErrorContextKeyVal, map[string]error{})
 }

act/common/line_writer.go

@@ -26,9 +26,7 @@ type lineWriter struct {
 
 // NewLineWriter creates a new instance of a line writer
 func NewLineWriter(handlers ...LineHandler) io.Writer {
-	w := new(lineWriter)
-	w.handlers = handlers
-	return w
+	return &lineWriter{handlers: handlers}
 }
 
 // FlushWriter flushes w if it implements Flusher. It is a no-op otherwise, so
@@ -39,24 +37,15 @@ func FlushWriter(w io.Writer) {
 	}
 }
 
-func (lw *lineWriter) Write(p []byte) (n int, err error) {
-	pBuf := bytes.NewBuffer(p)
-	written := 0
-	for {
-		line, err := pBuf.ReadString('\n')
-		w, _ := lw.buffer.WriteString(line)
-		written += w
-		if err != nil {
-			if err == io.EOF {
-				break
-			}
-			return written, err
-		}
+func (lw *lineWriter) Write(p []byte) (int, error) {
+	for line := range bytes.SplitAfterSeq(p, []byte{'\n'}) {
+		_, _ = lw.buffer.Write(line)
+		if len(line) > 0 && line[len(line)-1] == '\n' {
 			lw.handleLine(lw.buffer.String())
 			lw.buffer.Reset()
 		}
-
-	return written, nil
+	}
+	return len(p), nil
 }
 
 // Flush emits any buffered, not-yet-newline-terminated content as a final line.
@@ -72,8 +61,7 @@ func (lw *lineWriter) Flush() {
 
 func (lw *lineWriter) handleLine(line string) {
 	for _, h := range lw.handlers {
-		ok := h(line)
-		if !ok {
+		if !h(line) {
 			break
 		}
 	}

act/common/line_writer_test.go

@@ -9,24 +9,24 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
-func TestLineWriter(t *testing.T) {
-	lines := make([]string, 0)
-	lineHandler := func(s string) bool {
+func TestLineWriterBuffersAndFlushesLines(t *testing.T) {
+	var lines []string
+	lineWriter := NewLineWriter(func(s string) bool {
 		lines = append(lines, s)
 		return true
-	}
-
-	lineWriter := NewLineWriter(lineHandler)
+	})
 
 	assert := assert.New(t)
 	write := func(s string) {
 		n, err := lineWriter.Write([]byte(s))
-		assert.NoError(err) //nolint:testifylint // pre-existing issue from nektos/act
+		require.NoError(t, err)
 		assert.Equal(len(s), n, s)
 	}
 
+	write("")
 	write("hello")
 	write(" ")
 	write("world!!\nextra")
@@ -34,39 +34,17 @@ func TestLineWriter(t *testing.T) {
 	write(" line\n")
 	write("no newline here...")
 
-	assert.Len(lines, 4)
-	assert.Equal("hello world!!\n", lines[0])
-	assert.Equal("extra line\n", lines[1])
-	assert.Equal(" and another\n", lines[2])
-	assert.Equal("last line\n", lines[3])
-}
+	expected := []string{"hello world!!\n", "extra line\n", " and another\n", "last line\n"}
+	assert.Equal(expected, lines)
 
-func TestLineWriterFlush(t *testing.T) {
-	lines := make([]string, 0)
-	lineHandler := func(s string) bool {
-		lines = append(lines, s)
-		return true
-	}
-
-	lineWriter := NewLineWriter(lineHandler)
-
-	assert := assert.New(t)
-	_, err := lineWriter.Write([]byte("complete line\npartial line without newline"))
-	assert.NoError(err) //nolint:testifylint // pre-existing pattern from nektos/act
-
-	// Only the newline-terminated line is emitted before flushing.
-	assert.Equal([]string{"complete line\n"}, lines)
-
-	// Flushing emits the buffered, not-yet-terminated trailing line.
 	FlushWriter(lineWriter)
-	assert.Equal([]string{"complete line\n", "partial line without newline"}, lines)
+	expected = append(expected, "no newline here...")
+	assert.Equal(expected, lines)
 
-	// Flushing again is a no-op: nothing is buffered.
 	FlushWriter(lineWriter)
-	assert.Len(lines, 2)
+	assert.Equal(expected, lines)
 }
 
 func TestFlushWriterIgnoresNonFlusher(t *testing.T) {
-	// FlushWriter must be a safe no-op for writers that do not buffer lines.
 	assert.NotPanics(t, func() { FlushWriter(io.Discard) })
 }

act/common/logger.go

@@ -34,11 +34,9 @@ const loggerHandlerKeyVal = loggerHandlerKey("logging.Handler")
 // LoggerHandler returns the extra handler the job logger sends its records to, or nil.
 // It affects the job logger, not the global logger.
 func LoggerHandler(ctx context.Context) logging.Handler {
-	if h, ok := ctx.Value(loggerHandlerKeyVal).(logging.Handler); ok {
+	h, _ := ctx.Value(loggerHandlerKeyVal).(logging.Handler)
 	return h
 }
-	return nil
-}
 
 // WithLoggerHandler adds the extra handler for the job logger to the context
 func WithLoggerHandler(ctx context.Context, h logging.Handler) context.Context {

act/common/logging/console.go

@@ -9,6 +9,7 @@ import (
 	"fmt"
 	"io"
 	"log/slog"
+	"maps"
 	"reflect"
 	"runtime"
 	"slices"
@@ -94,11 +95,7 @@ func (h *consoleHandler) Handle(_ context.Context, r slog.Record) error {
 	}
 	prefixFieldClashes(data, caller != nil)
 
-	keys := make([]string, 0, len(data))
-	for k := range data {
-		keys = append(keys, k)
-	}
-	slices.Sort(keys)
+	keys := slices.Sorted(maps.Keys(data))
 
 	var b bytes.Buffer
 	if terminal && !opts.DisableColors {

act/common/logging/global.go

@@ -73,12 +73,10 @@ func Enabled(level Level) bool { return Default().Enabled(level) }
 
 func With(args ...any) *Logger          { return Default().With(args...) }
 func WithError(err error) *Logger       { return Default().WithError(err) }
-func Tracef(format string, args ...any) { Default().logf(LevelTrace, format, args) }
 func Debugf(format string, args ...any) { Default().logf(LevelDebug, format, args) }
 func Infof(format string, args ...any)  { Default().logf(LevelInfo, format, args) }
 func Warnf(format string, args ...any)  { Default().logf(LevelWarn, format, args) }
 func Errorf(format string, args ...any) { Default().logf(LevelError, format, args) }
-func Trace(args ...any)                 { Default().log(LevelTrace, args) }
 func Debug(args ...any)                 { Default().log(LevelDebug, args) }
 func Info(args ...any)                  { Default().log(LevelInfo, args) }
 func Warn(args ...any)                  { Default().log(LevelWarn, args) }

act/common/logging/logger.go

@@ -43,7 +43,6 @@ func (l *Logger) Enabled(level Level) bool {
 	return l.h.Enabled(l.ctx, level)
 }
 
-func (l *Logger) Tracef(format string, args ...any) { l.logf(LevelTrace, format, args) }
 func (l *Logger) Debugf(format string, args ...any) { l.logf(LevelDebug, format, args) }
 func (l *Logger) Infof(format string, args ...any)  { l.logf(LevelInfo, format, args) }
 func (l *Logger) Warnf(format string, args ...any)  { l.logf(LevelWarn, format, args) }

act/common/logging/logging_test.go

@@ -38,6 +38,7 @@ func (c countingStringer) String() string { *c.n++; return "x" }
 
 func TestLogger(t *testing.T) {
 	h := logtest.NewWithLevel(logging.LevelTrace)
+	assert.Nil(t, h.LastRecord())
 	l := logging.New(h)
 
 	l.With("job", "j1").WithError(errors.New("boom")).Warnf("step %d", 2)
@@ -53,12 +54,9 @@ func TestLogger(t *testing.T) {
 	assert.Equal(t, "j1", job)
 	errv, _ := recs[0].Attr("error")
 	require.EqualError(t, errv.(error), "boom")
-	assert.Equal(t, "a1 2b", recs[1].Message) // fmt.Sprint spacing
+	assert.Equal(t, "a1 2b", recs[1].Message)
 	bad, _ := recs[2].Attr("!BADKEY")
 	assert.Equal(t, 7, bad)
-
-	h.Reset()
-	assert.Nil(t, h.LastRecord())
 }
 
 func TestLoggerSkipsFormattingWhenDisabled(t *testing.T) {
@@ -117,10 +115,8 @@ func TestDefaultLevel(t *testing.T) {
 
 func TestSlogHandler(t *testing.T) {
 	var buf strings.Builder
-	slogger := slog.New(slog.NewTextHandler(&buf, &slog.HandlerOptions{Level: slog.LevelDebug}))
-	l := logging.New(logging.NewSlogHandler(slogger.Handler())).With("job", "j1")
+	l := logging.New(logging.NewSlogHandler(slog.NewTextHandler(&buf, &slog.HandlerOptions{Level: slog.LevelDebug}))).With("job", "j1")
 
-	l.Tracef("dropped")
 	assert.False(t, l.Enabled(logging.LevelTrace))
 	l.WithError(errors.New("boom")).Errorf("fail %s", "x")
 

act/common/logging/logtest/logtest.go

@@ -72,12 +72,6 @@ func (h *Handler) LastRecord() *logging.Record {
 	return &r
 }
 
-func (h *Handler) Reset() {
-	h.store.mu.Lock()
-	defer h.store.mu.Unlock()
-	h.store.records = nil
-}
-
 // SetDefault makes the process-wide Logger record into a new Handler until the test ends.
 func SetDefault(t testing.TB) *Handler {
 	t.Helper()

act/common/outbound_ip.go

@@ -12,7 +12,7 @@ import (
 
 type interfaceIP struct {
 	net.IP
-	net.Interface
+	Name string
 }
 
 // GetOutboundIP returns an outbound IP address of this machine.
@@ -43,7 +43,7 @@ func GetOutboundIP() net.IP {
 						ip = v.IP
 					}
 					if ip.IsGlobalUnicast() {
-						ips = append(ips, interfaceIP{ip, i})
+						ips = append(ips, interfaceIP{ip, i.Name})
 					}
 				}
 			}
@@ -59,29 +59,25 @@ func preferredIP(ips []interfaceIP) net.IP {
 		return nil
 	}
 	sort.Slice(ips, func(i, j int) bool {
-		ifi := ips[i].Interface
-		ifj := ips[j].Interface
+		first, second := ips[i], ips[j]
 
 		// ethernet is preferred
-		if vi, vj := strings.HasPrefix(ifi.Name, "e"), strings.HasPrefix(ifj.Name, "e"); vi != vj {
+		if vi, vj := strings.HasPrefix(first.Name, "e"), strings.HasPrefix(second.Name, "e"); vi != vj {
 			return vi
 		}
 
-		ipi := ips[i].IP
-		ipj := ips[j].IP
-
 		// IPv4 is preferred
-		if vi, vj := ipi.To4() != nil, ipj.To4() != nil; vi != vj {
+		if vi, vj := first.To4() != nil, second.To4() != nil; vi != vj {
 			return vi
 		}
 
 		// en0 is preferred to en1
-		if ifi.Name != ifj.Name {
-			return ifi.Name < ifj.Name
+		if first.Name != second.Name {
+			return first.Name < second.Name
 		}
 
 		// fallback
-		return ipi.String() < ipj.String()
+		return first.String() < second.String()
 	})
 	return ips[0].IP
 }

act/container/docker_build.go

@@ -46,9 +46,8 @@ func NewDockerBuildExecutor(input NewDockerBuildExecutorInput) common.Executor {
 
 		logger.Debugf("Building image from '%v'", input.ContextDir)
 
-		tags := []string{input.ImageTag}
 		options := client.ImageBuildOptions{
-			Tags:        tags,
+			Tags:        []string{input.ImageTag},
 			Remove:      true,
 			AuthConfigs: LoadDockerAuthConfigs(ctx),
 			Dockerfile:  input.Dockerfile,
@@ -76,11 +75,7 @@ func NewDockerBuildExecutor(input NewDockerBuildExecutorInput) common.Executor {
 		logger.Debugf("Creating image from context dir '%s' with tag '%s' and platform '%s'", input.ContextDir, input.ImageTag, input.Platform)
 		resp, err := cli.ImageBuild(ctx, buildContext, options)
 
-		err = logDockerResponse(logger, resp.Body, err != nil)
-		if err != nil {
-			return err
-		}
-		return nil
+		return logDockerResponse(logger, resp.Body, err != nil)
 	}
 }
 

act/container/docker_cli.go

@@ -4,12 +4,12 @@
 
 //go:build !(WITHOUT_DOCKER || !(linux || darwin || windows || netbsd))
 
-// This file is exact copy of https://github.com/docker/cli/blob/4a63305d74332de5ceba7fcbccbc3cbb7412f5ba/cli/command/container/opts.go with:
+// This file is based on https://github.com/docker/cli/blob/4a63305d74332de5ceba7fcbccbc3cbb7412f5ba/cli/command/container/opts.go with:
 // * appended with license information
 // * regexp and ParseVolume in place of the import-restricted internal/lazyregexp and internal/volumespec
 // * cdiDeviceName in place of the container-device-interface parser's IsQualifiedName
 // * jsontext in place of json.Compact for the seccomp profile
-// * invalidParameter from the package's errors.go, and convertPortSet/convertPortMap for the callers in docker_run.go
+// * invalidParameter from the package's errors.go, and convertPortSet/convertPortMap for nat port types
 //
 // docker/cli is licensed under the Apache License, Version 2.0.
 // See DOCKER_LICENSE for the full license text.
@@ -154,9 +154,6 @@ type containerOptions struct {
 	init                bool
 	umask               opts.UmaskOpt
 	annotations         *opts.MapOpts
-
-	Image string
-	Args  []string
 }
 
 // addFlags adds all command line flags that will be used by parse to the FlagSet
@@ -422,12 +419,7 @@ func parse(flags *pflag.FlagSet, copts *containerOptions, serverOS string) (*con
 		tmpfs[k] = v
 	}
 
-	var runCmd, entrypoint []string
-
-	if len(copts.Args) > 0 {
-		runCmd = copts.Args
-	}
-
+	var entrypoint []string
 	if copts.entrypoint != "" {
 		entrypoint = []string{copts.entrypoint}
 	} else if flags.Changed("entrypoint") {
@@ -448,37 +440,16 @@ func parse(flags *pflag.FlagSet, copts *containerOptions, serverOS string) (*con
 	if err != nil {
 		return nil, err
 	}
-	portBindings := network.PortMap{}
-	for port, bindings := range natPortBindings {
-		p, err := network.ParsePort(string(port))
+	portBindings, err := convertPortMap(natPortBindings)
 	if err != nil {
 		return nil, err
 	}
-		portBindings[p] = []network.PortBinding{}
-		for _, b := range bindings {
-			var hostIP netip.Addr
-			if b.HostIP != "" {
-				hostIP, err = netip.ParseAddr(b.HostIP)
-				if err != nil {
-					return nil, err
-				}
-			}
-			portBindings[p] = append(portBindings[p], network.PortBinding{
-				HostIP:   hostIP,
-				HostPort: b.HostPort,
-			})
-		}
-	}
 
 	// Add published ports as exposed ports.
-	exposedPorts := network.PortSet{}
-	for port := range ports {
-		p, err := network.ParsePort(string(port))
+	exposedPorts, err := convertPortSet(ports)
 	if err != nil {
 		return nil, err
 	}
-		exposedPorts[p] = struct{}{}
-	}
 
 	// Merge in exposed ports to the map of published ports
 	for _, e := range copts.expose.GetSlice() {
@@ -618,11 +589,10 @@ func parse(flags *pflag.FlagSet, copts *containerOptions, serverOS string) (*con
 
 	deviceRequests := copts.gpus.Value()
 	if len(cdiDeviceNames) > 0 {
-		cdiDeviceRequest := container.DeviceRequest{
+		deviceRequests = append(deviceRequests, container.DeviceRequest{
 			Driver:    "cdi",
 			DeviceIDs: cdiDeviceNames,
-		}
-		deviceRequests = append(deviceRequests, cdiDeviceRequest)
+		})
 	}
 
 	resources := container.Resources{
@@ -668,8 +638,6 @@ func parse(flags *pflag.FlagSet, copts *containerOptions, serverOS string) (*con
 		AttachStdout: attachStdout,
 		AttachStderr: attachStderr,
 		Env:          envVariables,
-		Cmd:          runCmd,
-		Image:        copts.Image,
 		Volumes:      volumes,
 		Entrypoint:   entrypoint,
 		WorkingDir:   copts.workingDir,
@@ -810,11 +778,9 @@ func parseNetworkOpts(copts *containerOptions) (map[string]*network.EndpointSett
 		// For backward compatibility: if no custom options are provided for the network,
 		// and only a single network is specified, omit the endpoint-configuration
 		// on the client (the daemon will still create it when creating the container)
-		if i == 0 && len(copts.netMode.Value()) == 1 {
-			if ep == nil || reflect.ValueOf(*ep).IsZero() {
+		if i == 0 && len(copts.netMode.Value()) == 1 && reflect.ValueOf(*ep).IsZero() {
 			continue
 		}
-		}
 		endpoints[n.Target] = ep
 	}
 	if hasUserDefined && hasNonUserDefined {
@@ -844,14 +810,12 @@ func applyContainerOptions(n *opts.NetworkAttachmentOpts, copts *containerOption
 		return invalidParameter(errors.New("conflicting options: cannot specify both --link-local-ip and per-network link-local IP addresses"))
 	}
 	if copts.aliases.Len() > 0 {
-		n.Aliases = make([]string, copts.aliases.Len())
-		copy(n.Aliases, copts.aliases.GetSlice())
+		n.Aliases = slices.Clone(copts.aliases.GetSlice())
 	}
 	// For a user-defined network, "--link" is an endpoint option, it creates an alias. But,
 	// for the default bridge it defines a legacy-link.
 	if container.NetworkMode(n.Target).IsUserDefined() && copts.links.Len() > 0 {
-		n.Links = make([]string, copts.links.Len())
-		copy(n.Links, copts.links.GetSlice())
+		n.Links = slices.Clone(copts.links.GetSlice())
 	}
 	if copts.ipv4Address != nil {
 		if ipv4, ok := netip.AddrFromSlice(copts.ipv4Address.To4()); ok {
@@ -890,7 +854,6 @@ func parseNetworkAttachmentOpt(ep opts.NetworkAttachmentOpts) (*network.Endpoint
 	}
 	epConfig.Aliases = append(epConfig.Aliases, ep.Aliases...)
 	if len(ep.DriverOpts) > 0 {
-		epConfig.DriverOpts = make(map[string]string)
 		epConfig.DriverOpts = ep.DriverOpts
 	}
 	if len(ep.Links) > 0 {
@@ -934,11 +897,10 @@ func convertToStandardNotation(ports []string) ([]string, error) {
 }
 
 func parseLoggingOpts(loggingDriver string, loggingOpts []string) (map[string]string, error) {
-	loggingOptsMap := opts.ConvertKVStringsToMap(loggingOpts)
 	if loggingDriver == "none" && len(loggingOpts) > 0 {
 		return map[string]string{}, fmt.Errorf("invalid logging opts for driver %s", loggingDriver)
 	}
-	return loggingOptsMap, nil
+	return opts.ConvertKVStringsToMap(loggingOpts), nil
 }
 
 // takes a local seccomp daemon, reads the file contents for sending to the daemon

act/container/docker_cli_test.go

@@ -2,7 +2,7 @@
 // Copyright 2022 The nektos/act Authors. All rights reserved.
 // SPDX-License-Identifier: MIT
 
-// This file is exact copy of https://github.com/docker/cli/blob/4a63305d74332de5ceba7fcbccbc3cbb7412f5ba/cli/command/container/opts_test.go with:
+// This file is based on https://github.com/docker/cli/blob/4a63305d74332de5ceba7fcbccbc3cbb7412f5ba/cli/command/container/opts_test.go with:
 // * appended with license information
 // * added tests for the locally changed parseDevice, validateDevice and invalidParameter
 //
@@ -69,7 +69,6 @@ func parseRun(args []string) (*container.Config, *container.HostConfig, *network
 	if err := flags.Parse(args); err != nil {
 		return nil, nil, nil, err
 	}
-	// TODO(dnephin): fix tests to accept ContainerConfig; see https://github.com/moby/moby/pull/31621
 	containerCfg, err := parse(flags, copts, runtime.GOOS)
 	if err != nil {
 		return nil, nil, nil, err
@@ -81,13 +80,12 @@ func setupRunFlags() (*pflag.FlagSet, *containerOptions) {
 	flags := pflag.NewFlagSet("run", pflag.ContinueOnError)
 	flags.SetOutput(io.Discard)
 	flags.Usage = nil
-	copts := addFlags(flags)
-	return flags, copts
+	return flags, addFlags(flags)
 }
 
 func mustParse(t *testing.T, args string) (*container.Config, *container.HostConfig, *networktypes.NetworkingConfig) {
 	t.Helper()
-	config, hostConfig, nwConfig, err := parseRun(append(strings.Split(args, " "), "ubuntu", "bash"))
+	config, hostConfig, nwConfig, err := parseRun(strings.Split(args, " "))
 	require.NoError(t, err)
 	return config, hostConfig, nwConfig
 }
@@ -179,53 +177,21 @@ func TestParseRunUmask(t *testing.T) {
 }
 
 func TestParseRunAttach(t *testing.T) {
-	tests := []struct {
+	for _, tc := range []struct {
 		input                 string
-		expected container.Config
+		stdin, stdout, stderr bool
+	}{
+		{"", false, true, true},
+		{"-i", true, true, true},
+		{"-a stdin", true, false, false},
+		{"-a stdin -a stdout", true, true, false},
+		{"-a stdin -a stdout -a stderr", true, true, true},
 	} {
-		{
-			input: "",
-			expected: container.Config{
-				AttachStdout: true,
-				AttachStderr: true,
-			},
-		},
-		{
-			input: "-i",
-			expected: container.Config{
-				AttachStdin:  true,
-				AttachStdout: true,
-				AttachStderr: true,
-			},
-		},
-		{
-			input: "-a stdin",
-			expected: container.Config{
-				AttachStdin: true,
-			},
-		},
-		{
-			input: "-a stdin -a stdout",
-			expected: container.Config{
-				AttachStdin:  true,
-				AttachStdout: true,
-			},
-		},
-		{
-			input: "-a stdin -a stdout -a stderr",
-			expected: container.Config{
-				AttachStdin:  true,
-				AttachStdout: true,
-				AttachStderr: true,
-			},
-		},
-	}
-	for _, tc := range tests {
 		t.Run(tc.input, func(t *testing.T) {
 			config, _, _ := mustParse(t, tc.input)
-			require.Equal(t, tc.expected.AttachStdin, config.AttachStdin)
-			require.Equal(t, tc.expected.AttachStdout, config.AttachStdout)
-			require.Equal(t, tc.expected.AttachStderr, config.AttachStderr)
+			require.Equal(t, tc.stdin, config.AttachStdin)
+			require.Equal(t, tc.stdout, config.AttachStdout)
+			require.Equal(t, tc.stderr, config.AttachStderr)
 		})
 	}
 }
@@ -276,104 +242,47 @@ func TestParseRunWithInvalidArgs(t *testing.T) {
 	}
 }
 
-func TestParseWithVolumes(t *testing.T) { //nolint:gocyclo // verbatim copy from docker/cli tests
-	// A single volume
-	arr, tryit := setupPlatformVolume([]string{`/tmp`}, []string{`c:\tmp`})
-	if config, hostConfig, _ := mustParse(t, tryit); hostConfig.Binds != nil {
-		t.Fatalf("Error parsing volume flags, %q should not mount-bind anything. Received %v", tryit, hostConfig.Binds)
-	} else if _, exists := config.Volumes[arr[0]]; !exists {
-		t.Fatalf("Error parsing volume flags, %q is missing from volumes. Received %v", tryit, config.Volumes)
-	}
-
-	// Two volumes
-	arr, tryit = setupPlatformVolume([]string{`/tmp`, `/var`}, []string{`c:\tmp`, `c:\var`})
-	if config, hostConfig, _ := mustParse(t, tryit); hostConfig.Binds != nil {
-		t.Fatalf("Error parsing volume flags, %q should not mount-bind anything. Received %v", tryit, hostConfig.Binds)
-	} else if _, exists := config.Volumes[arr[0]]; !exists {
-		t.Fatalf("Error parsing volume flags, %s is missing from volumes. Received %v", arr[0], config.Volumes)
-	} else if _, exists := config.Volumes[arr[1]]; !exists {
-		t.Fatalf("Error parsing volume flags, %s is missing from volumes. Received %v", arr[1], config.Volumes)
-	}
-
-	// A single bind mount
-	arr, tryit = setupPlatformVolume([]string{`/hostTmp:/containerTmp`}, []string{os.Getenv("TEMP") + `:c:\containerTmp`})
-	if config, hostConfig, _ := mustParse(t, tryit); hostConfig.Binds == nil || hostConfig.Binds[0] != arr[0] {
-		t.Fatalf("Error parsing volume flags, %q should mount-bind the path before the colon into the path after the colon. Received %v %v", arr[0], hostConfig.Binds, config.Volumes)
-	}
-
-	// Two bind mounts.
-	arr, tryit = setupPlatformVolume([]string{`/hostTmp:/containerTmp`, `/hostVar:/containerVar`}, []string{os.Getenv("ProgramData") + `:c:\ContainerPD`, os.Getenv("TEMP") + `:c:\containerTmp`})
-	if _, hostConfig, _ := mustParse(t, tryit); hostConfig.Binds == nil || compareRandomizedStrings(hostConfig.Binds[0], hostConfig.Binds[1], arr[0], arr[1]) != nil {
-		t.Fatalf("Error parsing volume flags, `%s and %s` did not mount-bind correctly. Received %v", arr[0], arr[1], hostConfig.Binds)
-	}
-
-	// Two bind mounts, first read-only, second read-write.
-	// TODO Windows: The Windows version uses read-write as that's the only mode it supports. Can change this post TP4
-	arr, tryit = setupPlatformVolume(
-		[]string{`/hostTmp:/containerTmp:ro`, `/hostVar:/containerVar:rw`},
-		[]string{os.Getenv("TEMP") + `:c:\containerTmp:rw`, os.Getenv("ProgramData") + `:c:\ContainerPD:rw`})
-	if _, hostConfig, _ := mustParse(t, tryit); hostConfig.Binds == nil || compareRandomizedStrings(hostConfig.Binds[0], hostConfig.Binds[1], arr[0], arr[1]) != nil {
-		t.Fatalf("Error parsing volume flags, `%s and %s` did not mount-bind correctly. Received %v", arr[0], arr[1], hostConfig.Binds)
-	}
-
-	// Similar to previous test but with alternate modes which are only supported by Linux
-	if runtime.GOOS != "windows" {
-		arr, tryit = setupPlatformVolume([]string{`/hostTmp:/containerTmp:ro,Z`, `/hostVar:/containerVar:rw,Z`}, []string{})
-		if _, hostConfig, _ := mustParse(t, tryit); hostConfig.Binds == nil || compareRandomizedStrings(hostConfig.Binds[0], hostConfig.Binds[1], arr[0], arr[1]) != nil {
-			t.Fatalf("Error parsing volume flags, `%s and %s` did not mount-bind correctly. Received %v", arr[0], arr[1], hostConfig.Binds)
-		}
-
-		arr, tryit = setupPlatformVolume([]string{`/hostTmp:/containerTmp:Z`, `/hostVar:/containerVar:z`}, []string{})
-		if _, hostConfig, _ := mustParse(t, tryit); hostConfig.Binds == nil || compareRandomizedStrings(hostConfig.Binds[0], hostConfig.Binds[1], arr[0], arr[1]) != nil {
-			t.Fatalf("Error parsing volume flags, `%s and %s` did not mount-bind correctly. Received %v", arr[0], arr[1], hostConfig.Binds)
-		}
-	}
-
-	// One bind mount and one volume
-	arr, tryit = setupPlatformVolume([]string{`/hostTmp:/containerTmp`, `/containerVar`}, []string{os.Getenv("TEMP") + `:c:\containerTmp`, `c:\containerTmp`})
-	if config, hostConfig, _ := mustParse(t, tryit); hostConfig.Binds == nil || len(hostConfig.Binds) > 1 || hostConfig.Binds[0] != arr[0] {
-		t.Fatalf("Error parsing volume flags, %s and %s should only one and only one bind mount %s. Received %s", arr[0], arr[1], arr[0], hostConfig.Binds)
-	} else if _, exists := config.Volumes[arr[1]]; !exists {
-		t.Fatalf("Error parsing volume flags %s and %s. %s is missing from volumes. Received %v", arr[0], arr[1], arr[1], config.Volumes)
-	}
-
-	// Root to non-c: drive letter (Windows specific)
+func TestParseWithVolumes(t *testing.T) {
+	for _, tc := range []struct {
+		unix, windows []string
+		binds         int
+	}{
+		{[]string{`/tmp`}, []string{`c:\tmp`}, 0},
+		{[]string{`/tmp`, `/var`}, []string{`c:\tmp`, `c:\var`}, 0},
+		{[]string{`/hostTmp:/containerTmp`}, []string{os.Getenv("TEMP") + `:c:\containerTmp`}, 1},
+		{[]string{`/hostTmp:/containerTmp`, `/hostVar:/containerVar`}, []string{os.Getenv("ProgramData") + `:c:\ContainerPD`, os.Getenv("TEMP") + `:c:\containerTmp`}, 2},
+		{[]string{`/hostTmp:/containerTmp:ro`, `/hostVar:/containerVar:rw`}, []string{os.Getenv("TEMP") + `:c:\containerTmp:rw`, os.Getenv("ProgramData") + `:c:\ContainerPD:rw`}, 2},
+		{[]string{`/hostTmp:/containerTmp:ro,Z`, `/hostVar:/containerVar:rw,Z`}, nil, 2},
+		{[]string{`/hostTmp:/containerTmp:Z`, `/hostVar:/containerVar:z`}, nil, 2},
+		{[]string{`/hostTmp:/containerTmp`, `/containerVar`}, []string{os.Getenv("TEMP") + `:c:\containerTmp`, `c:\containerTmp`}, 1},
+		{nil, []string{os.Getenv("SystemDrive") + `\:d:`}, 1},
+	} {
+		volumes := tc.unix
 		if runtime.GOOS == "windows" {
-		arr, tryit = setupPlatformVolume([]string{}, []string{os.Getenv("SystemDrive") + `\:d:`})
-		if config, hostConfig, _ := mustParse(t, tryit); hostConfig.Binds == nil || len(hostConfig.Binds) > 1 || hostConfig.Binds[0] != arr[0] || len(config.Volumes) != 0 {
-			t.Fatalf("Error parsing %s. Should have a single bind mount and no volumes", arr[0])
+			volumes = tc.windows
 		}
+		if len(volumes) == 0 {
+			continue
 		}
+		t.Run(strings.Join(volumes, ","), func(t *testing.T) {
+			var args []string
+			for _, volume := range volumes {
+				args = append(args, "-v", volume)
 			}
-
-// setupPlatformVolume takes two arrays of volume specs - a Unix style
-// spec and a Windows style spec. Depending on the platform being unit tested,
-// it returns one of them, along with a volume string that would be passed
-// on the docker CLI (e.g. -v /bar -v /foo).
-func setupPlatformVolume(u, w []string) ([]string, string) {
-	var a []string
-	if runtime.GOOS == "windows" {
-		a = w
+			config, hostConfig, _, err := parseRun(args)
+			require.NoError(t, err)
+			if tc.binds == 0 {
+				assert.Nil(t, hostConfig.Binds)
 			} else {
-		a = u
+				assert.ElementsMatch(t, volumes[:tc.binds], hostConfig.Binds)
 			}
-	s := ""
-	for _, v := range a {
-		s = s + "-v " + v + " "
+			expectedVolumes := map[string]struct{}{}
+			for _, volume := range volumes[tc.binds:] {
+				expectedVolumes[volume] = struct{}{}
 			}
-	return a, s
+			assert.Equal(t, expectedVolumes, config.Volumes)
+		})
 	}
-
-// check if (a == c && b == d) || (a == d && b == c)
-// because maps are randomized
-func compareRandomizedStrings(a, b, c, d string) error {
-	if a == c && b == d {
-		return nil
-	}
-	if a == d && b == c {
-		return nil
-	}
-	return errors.New("strings don't match")
 }
 
 // Simple parse with MacAddress validation
@@ -414,46 +323,29 @@ func TestParseWithMemorySwap(t *testing.T) {
 }
 
 func TestParseHostname(t *testing.T) {
-	validHostnames := map[string]string{
-		"hostname":    "hostname",
-		"host-name":   "host-name",
-		"hostname123": "hostname123",
-		"123hostname": "123hostname",
-		"hostname-of-63-bytes-long-should-be-valid-and-without-any-error": "hostname-of-63-bytes-long-should-be-valid-and-without-any-error",
-	}
-	hostnameWithDomain := "--hostname=hostname.domainname"
-	hostnameWithDomainTld := "--hostname=hostname.domainname.tld"
-	for hostname, expectedHostname := range validHostnames {
-		if config, _, _ := mustParse(t, "--hostname="+hostname); config.Hostname != expectedHostname {
-			t.Fatalf("Expected the config to have 'hostname' as %q, got %q", expectedHostname, config.Hostname)
-		}
-	}
-	if config, _, _ := mustParse(t, hostnameWithDomain); config.Hostname != "hostname.domainname" || config.Domainname != "" {
-		t.Fatalf("Expected the config to have 'hostname' as hostname.domainname, got %q", config.Hostname)
-	}
-	if config, _, _ := mustParse(t, hostnameWithDomainTld); config.Hostname != "hostname.domainname.tld" || config.Domainname != "" {
-		t.Fatalf("Expected the config to have 'hostname' as hostname.domainname.tld, got %q", config.Hostname)
-	}
-}
-
-func TestParseHostnameDomainname(t *testing.T) {
-	validDomainnames := map[string]string{
-		"domainname":    "domainname",
-		"domain-name":   "domain-name",
-		"domainname123": "domainname123",
-		"123domainname": "123domainname",
-		"domainname-63-bytes-long-should-be-valid-and-without-any-errors": "domainname-63-bytes-long-should-be-valid-and-without-any-errors",
-	}
-	for domainname, expectedDomainname := range validDomainnames {
-		if config, _, _ := mustParse(t, "--domainname="+domainname); config.Domainname != expectedDomainname {
-			t.Fatalf("Expected the config to have 'domainname' as %q, got %q", expectedDomainname, config.Domainname)
-		}
-	}
-	if config, _, _ := mustParse(t, "--hostname=some.prefix --domainname=domainname"); config.Hostname != "some.prefix" || config.Domainname != "domainname" {
-		t.Fatalf("Expected the config to have 'hostname' as 'some.prefix' and 'domainname' as 'domainname', got %q and %q", config.Hostname, config.Domainname)
-	}
-	if config, _, _ := mustParse(t, "--hostname=another-prefix --domainname=domainname.tld"); config.Hostname != "another-prefix" || config.Domainname != "domainname.tld" {
-		t.Fatalf("Expected the config to have 'hostname' as 'another-prefix' and 'domainname' as 'domainname.tld', got %q and %q", config.Hostname, config.Domainname)
+	for _, tc := range []struct {
+		input, hostname, domainname string
+	}{
+		{"--hostname=hostname", "hostname", ""},
+		{"--hostname=host-name", "host-name", ""},
+		{"--hostname=hostname123", "hostname123", ""},
+		{"--hostname=123hostname", "123hostname", ""},
+		{"--hostname=hostname-of-63-bytes-long-should-be-valid-and-without-any-error", "hostname-of-63-bytes-long-should-be-valid-and-without-any-error", ""},
+		{"--hostname=hostname.domainname", "hostname.domainname", ""},
+		{"--hostname=hostname.domainname.tld", "hostname.domainname.tld", ""},
+		{"--domainname=domainname", "", "domainname"},
+		{"--domainname=domain-name", "", "domain-name"},
+		{"--domainname=domainname123", "", "domainname123"},
+		{"--domainname=123domainname", "", "123domainname"},
+		{"--domainname=domainname-63-bytes-long-should-be-valid-and-without-any-errors", "", "domainname-63-bytes-long-should-be-valid-and-without-any-errors"},
+		{"--hostname=some.prefix --domainname=domainname", "some.prefix", "domainname"},
+		{"--hostname=another-prefix --domainname=domainname.tld", "another-prefix", "domainname.tld"},
+	} {
+		t.Run(tc.input, func(t *testing.T) {
+			config, _, _ := mustParse(t, tc.input)
+			require.Equal(t, tc.hostname, config.Hostname)
+			require.Equal(t, tc.domainname, config.Domainname)
+		})
 	}
 }
 
@@ -497,9 +389,15 @@ func TestParseWithExpose(t *testing.T) {
 	})
 
 	t.Run("merge with published", func(t *testing.T) {
-		// Merge with actual published port
-		config, _, _, err := parseRun([]string{"--publish=80", "--expose=80-81/tcp", "img", "cmd"})
+		config, hostConfig, _, err := parseRun([]string{"--publish=80", "--publish=127.0.0.1:8080:80", "--publish=[::1]:8081:80", "--expose=80-81/tcp"})
 		require.NoError(t, err)
+		assert.Equal(t, networktypes.PortMap{
+			networktypes.MustParsePort("80/tcp"): {
+				{},
+				{HostIP: netip.MustParseAddr("127.0.0.1"), HostPort: "8080"},
+				{HostIP: netip.MustParseAddr("::1"), HostPort: "8081"},
+			},
+		}, hostConfig.PortBindings)
 		assert.Len(t, config.ExposedPorts, 2)
 		ports := []networktypes.Port{networktypes.MustParsePort("80/tcp"), networktypes.MustParsePort("81/tcp")}
 		for _, port := range ports {
@@ -984,41 +882,29 @@ func TestParseRestartPolicyAutoRemove(t *testing.T) {
 }
 
 func TestParseHealth(t *testing.T) {
-	checkOk := func(args ...string) *container.HealthConfig {
-		config, _, _, err := parseRun(args)
-		if err != nil {
-			t.Fatalf("%#v: %v", args, err)
+	for _, tc := range []struct {
+		args     []string
+		expected *container.HealthConfig
+		wantErr  string
+	}{
+		{[]string{"--no-healthcheck", "img", "cmd"}, &container.HealthConfig{Test: []string{"NONE"}}, ""},
+		{[]string{"--health-cmd=/check.sh -q", "img", "cmd"}, &container.HealthConfig{Test: []string{"CMD-SHELL", "/check.sh -q"}}, ""},
+		{[]string{"--no-healthcheck", "--health-cmd=/check.sh -q", "img", "cmd"}, nil, "--no-healthcheck conflicts with --health-* options"},
+		{
+			[]string{"--health-timeout=2s", "--health-retries=3", "--health-interval=4.5s", "--health-start-period=5s", "--health-start-interval=1s", "img", "cmd"},
+			&container.HealthConfig{Timeout: 2 * time.Second, Retries: 3, Interval: 4500 * time.Millisecond, StartPeriod: 5 * time.Second, StartInterval: time.Second},
+			"",
+		},
+	} {
+		t.Run(strings.Join(tc.args, " "), func(t *testing.T) {
+			config, _, _, err := parseRun(tc.args)
+			if tc.wantErr != "" {
+				require.EqualError(t, err, tc.wantErr)
+			} else {
+				require.NoError(t, err)
+				require.Equal(t, tc.expected, config.Healthcheck)
 			}
-		return config.Healthcheck
-	}
-	checkError := func(expected string, args ...string) {
-		config, _, _, err := parseRun(args)
-		if err == nil {
-			t.Fatalf("Expected error, but got %#v", config)
-		}
-		if err.Error() != expected {
-			t.Fatalf("Expected %#v, got %#v", expected, err)
-		}
-	}
-	health := checkOk("--no-healthcheck", "img", "cmd")
-	if health == nil || len(health.Test) != 1 || health.Test[0] != "NONE" {
-		t.Fatalf("--no-healthcheck failed: %#v", health)
-	}
-
-	health = checkOk("--health-cmd=/check.sh -q", "img", "cmd")
-	if len(health.Test) != 2 || health.Test[0] != "CMD-SHELL" || health.Test[1] != "/check.sh -q" {
-		t.Fatalf("--health-cmd: got %#v", health.Test)
-	}
-	if health.Timeout != 0 {
-		t.Fatalf("--health-cmd: timeout = %s", health.Timeout)
-	}
-
-	checkError("--no-healthcheck conflicts with --health-* options",
-		"--no-healthcheck", "--health-cmd=/check.sh -q", "img", "cmd")
-
-	health = checkOk("--health-timeout=2s", "--health-retries=3", "--health-interval=4.5s", "--health-start-period=5s", "--health-start-interval=1s", "img", "cmd")
-	if health.Timeout != 2*time.Second || health.Retries != 3 || health.Interval != 4500*time.Millisecond || health.StartPeriod != 5*time.Second || health.StartInterval != 1*time.Second {
-		t.Fatalf("--health-*: got %#v", health)
+		})
 	}
 }
 
@@ -1037,82 +923,53 @@ func TestParseLoggingOpts(t *testing.T) {
 	}
 }
 
-func TestParseEnvfileVariables(t *testing.T) { //nolint:dupl // verbatim copy from docker/cli tests
-	expErr := "--env-file: open nonexistent: no such file or directory"
+func TestParseFileVariables(t *testing.T) {
+	for _, tc := range []struct {
+		flag, file, key string
+	}{
+		{"env", "valid.env", "ENV"},
+		{"label", "valid.label", "LABEL"},
+	} {
+		t.Run(tc.flag, func(t *testing.T) {
+			expectedError := "no such file or directory"
 			if runtime.GOOS == "windows" {
-		expErr = "--env-file: open nonexistent: The system cannot find the file specified."
+				expectedError = "The system cannot find the file specified."
 			}
-	// env ko
-	if _, _, _, err := parseRun([]string{"--env-file=nonexistent", "img", "cmd"}); err == nil || err.Error() != expErr {
-		t.Fatalf("Expected an error with message '%s', got %v", expErr, err)
+			_, _, _, err := parseRun([]string{"--" + tc.flag + "-file=nonexistent"})
+			require.EqualError(t, err, "--"+tc.flag+"-file: open nonexistent: "+expectedError)
+			for _, extra := range []bool{false, true} {
+				args := []string{"--" + tc.flag + "-file=testdata/" + tc.file}
+				expected := []string{tc.key + "1=value1"}
+				if extra {
+					args = append(args, "--"+tc.flag+"="+tc.key+"2=value2")
+					expected = append(expected, tc.key+"2=value2")
 				}
-	// env ok
-	config, _, _, err := parseRun([]string{"--env-file=testdata/valid.env", "img", "cmd"})
-	if err != nil {
-		t.Fatal(err)
+				config, _, _, err := parseRun(args)
+				require.NoError(t, err)
+				if tc.flag == "env" {
+					assert.Equal(t, expected, config.Env)
+				} else {
+					assert.Len(t, config.Labels, len(expected))
+					for _, label := range expected {
+						key, value, _ := strings.Cut(label, "=")
+						assert.Equal(t, value, config.Labels[key])
 					}
-	if len(config.Env) != 1 || config.Env[0] != "ENV1=value1" {
-		t.Fatalf("Expected a config with [ENV1=value1], got %v", config.Env)
 				}
-	config, _, _, err = parseRun([]string{"--env-file=testdata/valid.env", "--env=ENV2=value2", "img", "cmd"})
-	if err != nil {
-		t.Fatal(err)
 			}
-	if len(config.Env) != 2 || config.Env[0] != "ENV1=value1" || config.Env[1] != "ENV2=value2" {
-		t.Fatalf("Expected a config with [ENV1=value1 ENV2=value2], got %v", config.Env)
+		})
 	}
 }
 
 func TestParseEnvfileVariablesWithBOMUnicode(t *testing.T) {
-	// UTF8 with BOM
-	config, _, _, err := parseRun([]string{"--env-file=testdata/utf8.env", "img", "cmd"})
-	if err != nil {
-		t.Fatal(err)
-	}
-	env := []string{"FOO=BAR", "HELLO=" + string([]byte{0xe6, 0x82, 0xa8, 0xe5, 0xa5, 0xbd}), "BAR=FOO"}
-	if len(config.Env) != len(env) {
-		t.Fatalf("Expected a config with %d env variables, got %v: %v", len(env), len(config.Env), config.Env)
-	}
-	for i, v := range env {
-		if config.Env[i] != v {
-			t.Fatalf("Expected a config with [%s], got %v", v, []byte(config.Env[i]))
-		}
-	}
+	config, _, _, err := parseRun([]string{"--env-file=testdata/utf8.env"})
+	require.NoError(t, err)
+	assert.Equal(t, []string{"FOO=BAR", "HELLO=您好", "BAR=FOO"}, config.Env)
 
-	// UTF16 with BOM
-	e := "invalid utf8 bytes at line"
-	if _, _, _, err := parseRun([]string{"--env-file=testdata/utf16.env", "img", "cmd"}); err == nil || !strings.Contains(err.Error(), e) {
-		t.Fatalf("Expected an error with message '%s', got %v", e, err)
-	}
-	// UTF16BE with BOM
-	if _, _, _, err := parseRun([]string{"--env-file=testdata/utf16be.env", "img", "cmd"}); err == nil || !strings.Contains(err.Error(), e) {
-		t.Fatalf("Expected an error with message '%s', got %v", e, err)
-	}
-}
-
-func TestParseLabelfileVariables(t *testing.T) { //nolint:dupl // verbatim copy from docker/cli tests
-	expErr := "--label-file: open nonexistent: no such file or directory"
-	if runtime.GOOS == "windows" {
-		expErr = "--label-file: open nonexistent: The system cannot find the file specified."
-	}
-	// label ko
-	if _, _, _, err := parseRun([]string{"--label-file=nonexistent", "img", "cmd"}); err == nil || err.Error() != expErr {
-		t.Fatalf("Expected an error with message '%s', got %v", expErr, err)
-	}
-	// label ok
-	config, _, _, err := parseRun([]string{"--label-file=testdata/valid.label", "img", "cmd"})
-	if err != nil {
-		t.Fatal(err)
-	}
-	if len(config.Labels) != 1 || config.Labels["LABEL1"] != "value1" {
-		t.Fatalf("Expected a config with [LABEL1:value1], got %v", config.Labels)
-	}
-	config, _, _, err = parseRun([]string{"--label-file=testdata/valid.label", "--label=LABEL2=value2", "img", "cmd"})
-	if err != nil {
-		t.Fatal(err)
-	}
-	if len(config.Labels) != 2 || config.Labels["LABEL1"] != "value1" || config.Labels["LABEL2"] != "value2" {
-		t.Fatalf("Expected a config with [LABEL1:value1 LABEL2:value2], got %v", config.Labels)
+	for _, file := range []string{"utf16.env", "utf16be.env"} {
+		t.Run(file, func(t *testing.T) {
+			_, _, _, err := parseRun([]string{"--env-file=testdata/" + file})
+			require.ErrorContains(t, err, "invalid utf8 bytes at line")
+		})
 	}
 }
 

act/container/docker_images.go

@@ -58,18 +58,14 @@ func RemoveImage(ctx context.Context, imageName string, force, pruneChildren boo
 	}
 	defer cli.Close()
 
-	if _, err := cli.ImageInspect(ctx, imageName); cerrdefs.IsNotFound(err) {
+	if _, err = cli.ImageRemove(ctx, imageName, client.ImageRemoveOptions{
+		Force:         force,
+		PruneChildren: pruneChildren,
+	}); cerrdefs.IsNotFound(err) {
 		return false, nil
 	} else if err != nil {
 		return false, err
 	}
 
-	if _, err = cli.ImageRemove(ctx, imageName, client.ImageRemoveOptions{
-		Force:         force,
-		PruneChildren: pruneChildren,
-	}); err != nil {
-		return false, err
-	}
-
 	return true, nil
 }

act/container/docker_images_test.go

@@ -66,4 +66,11 @@ func TestImageExistsLocally(t *testing.T) {
 	wrongPlatform, err := ImageExistsLocally(ctx, amd64Ref, "linux/arm64")
 	assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act
 	assert.False(t, wrongPlatform)
+
+	removed, err := RemoveImage(ctx, amd64Ref, false, false)
+	require.NoError(t, err)
+	assert.True(t, removed)
+	removed, err = RemoveImage(ctx, amd64Ref, false, false)
+	require.NoError(t, err)
+	assert.False(t, removed)
 }

act/container/docker_logger.go

@@ -33,18 +33,9 @@ func logDockerResponse(logger *logging.Logger, dockerResponse io.ReadCloser, isE
 	defer dockerResponse.Close()
 
 	scanner := bufio.NewScanner(dockerResponse)
-	msg := dockerMessage{}
-
 	for scanner.Scan() {
 		line := scanner.Bytes()
-
-		msg.ID = ""
-		msg.Stream = ""
-		msg.Error = ""
-		msg.ErrorDetail.Message = ""
-		msg.Status = ""
-		msg.Progress = ""
-
+		var msg dockerMessage
 		if err := json.Unmarshal(line, &msg); err != nil {
 			writeLog(logger, false, "Unable to unmarshal line [%s] ==> %v", string(line), err)
 			continue

act/container/docker_platform_test.go

@@ -6,6 +6,7 @@ package container
 import (
 	"testing"
 
+	specs "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -17,32 +18,17 @@ func TestParsePlatform(t *testing.T) {
 		assert.Nil(t, got)
 	})
 
-	t.Run("os/arch", func(t *testing.T) {
-		got, err := parsePlatform("linux/amd64")
+	for input, want := range map[string]specs.Platform{
+		"linux/amd64":    {OS: "linux", Architecture: "amd64"},
+		"linux/arm/v7":   {OS: "linux", Architecture: "arm", Variant: "v7"},
+		"Linux/AMD64/V8": {OS: "linux", Architecture: "amd64", Variant: "v8"},
+	} {
+		t.Run(input, func(t *testing.T) {
+			got, err := parsePlatform(input)
 			require.NoError(t, err)
-		require.NotNil(t, got)
-		assert.Equal(t, "linux", got.OS)
-		assert.Equal(t, "amd64", got.Architecture)
-		assert.Empty(t, got.Variant)
-	})
-
-	t.Run("os/arch/variant", func(t *testing.T) {
-		got, err := parsePlatform("linux/arm/v7")
-		require.NoError(t, err)
-		require.NotNil(t, got)
-		assert.Equal(t, "linux", got.OS)
-		assert.Equal(t, "arm", got.Architecture)
-		assert.Equal(t, "v7", got.Variant)
-	})
-
-	t.Run("input is lowercased", func(t *testing.T) {
-		got, err := parsePlatform("Linux/AMD64/V8")
-		require.NoError(t, err)
-		require.NotNil(t, got)
-		assert.Equal(t, "linux", got.OS)
-		assert.Equal(t, "amd64", got.Architecture)
-		assert.Equal(t, "v8", got.Variant)
+			assert.Equal(t, &want, got)
 		})
+	}
 
 	for _, bad := range []string{
 		"amd64",

act/container/docker_proxy.go

@@ -48,6 +48,13 @@ var (
 	rawStreamPath = regexp.MustCompile(`^(/v[0-9.]+)?/(containers/[^/]+/attach|exec/[^/]+/start)$`)
 )
 
+var dockerProxyDirectories sync.Map
+
+type dockerProxyDirectory struct {
+	dir, daemonDir string
+	socket         os.FileInfo
+}
+
 func NewDockerProxy(ctx context.Context, job string) *DockerProxy {
 	if host := os.Getenv("DOCKER_HOST"); runtime.GOOS != "linux" || host != "" && !strings.HasPrefix(host, "unix://") {
 		return nil
@@ -63,17 +70,26 @@ func NewDockerProxy(ctx context.Context, job string) *DockerProxy {
 	if !ok {
 		return nil
 	}
-	if info, err := os.Stat(daemonSocket); err != nil || info.Mode()&os.ModeSocket == 0 {
+	socket, err := os.Stat(daemonSocket)
+	if err != nil || socket.Mode()&os.ModeSocket == 0 {
 		return nil
 	}
-	dir, daemonDir := runnerContainerWorkdir(probeCtx, cli)
-	seen := daemonDir != ""
+	workdir, _ := os.Getwd()
+	key := [3]string{cli.DaemonHost(), workdir, os.TempDir()}
+	cached, _ := dockerProxyDirectories.Load(key)
+	directory, seen := cached.(dockerProxyDirectory)
+	seen = seen && os.SameFile(directory.socket, socket) // daemon replacement invalidates cached paths
+	dir, daemonDir := directory.dir, directory.daemonDir
+	if !seen {
+		dir, daemonDir = runnerContainerWorkdir(probeCtx, cli)
+		seen = daemonDir != ""
 		if !seen {
 			if dir, err = filepath.Abs(os.TempDir()); err == nil {
 				daemonDir = dir
 				seen, err = daemonSeesDir(probeCtx, cli, dir, daemonDir)
 			}
 		}
+	}
 	if err != nil {
 		common.Logger(ctx).Infof("docker proxy probe failed, jobs get the daemon socket directly: %v", err)
 		return nil
@@ -87,9 +103,11 @@ func NewDockerProxy(ctx context.Context, job string) *DockerProxy {
 	}
 	proxy, err := StartDockerProxy(daemonSocket, dir, job)
 	if err != nil {
+		dockerProxyDirectories.Delete(key)
 		common.Logger(ctx).Warnf("docker proxy not started, the job gets the daemon socket directly: %v", err)
 		return nil
 	}
+	dockerProxyDirectories.Store(key, dockerProxyDirectory{dir, daemonDir, socket})
 	proxy.Socket = daemonDir + strings.TrimPrefix(proxy.Socket, dir)
 	return proxy
 }

act/container/docker_proxy_test.go

@@ -20,6 +20,7 @@ import (
 	"runtime"
 	"strconv"
 	"strings"
+	"sync/atomic"
 	"testing"
 	"time"
 
@@ -68,6 +69,7 @@ func TestDockerProxy(t *testing.T) {
 	bodies := make(chan []byte, 8)
 	createStarted, releaseCreate := make(chan struct{}), make(chan struct{})
 	streamsDone := make(chan struct{}, 2)
+	var probes atomic.Int32
 	daemonSocket := filepath.Join(shortTempDir(t), "d.sock")
 	listener, err := net.Listen("unix", daemonSocket)
 	require.NoError(t, err)
@@ -75,12 +77,28 @@ func TestDockerProxy(t *testing.T) {
 		switch {
 		case strings.HasSuffix(r.URL.Path, "/create"):
 			body, _ := io.ReadAll(r.Body)
+			if bytes.Contains(body, []byte(`"Image":"probe-image"`)) {
+				_, _ = w.Write([]byte(`{"Id":"probe"}`))
+				return
+			}
 			if r.URL.RawQuery == "wait" {
 				close(createStarted)
 				<-releaseCreate
 			}
 			bodies <- body
 			w.WriteHeader(http.StatusCreated)
+		case strings.HasSuffix(r.URL.Path, "/images/json"):
+			if probes.Add(1) == 1 {
+				http.Error(w, "probe unavailable", http.StatusServiceUnavailable)
+				return
+			}
+			_, _ = w.Write([]byte(`[{"Id":"probe-image"}]`))
+		case strings.HasSuffix(r.URL.Path, "/json"):
+			http.NotFound(w, r)
+		case strings.HasSuffix(r.URL.Path, "/archive"):
+			w.Header().Set("X-Docker-Container-Path-Stat", "e30=")
+		case r.Method == http.MethodDelete:
+			w.WriteHeader(http.StatusNoContent)
 		case r.URL.Path == "/exec/detached/start" || r.URL.Path == "/exec/error/start":
 			if r.URL.Path == "/exec/error/start" {
 				w.Header().Set("Content-Type", "application/vnd.docker.raw-stream")
@@ -211,6 +229,31 @@ func TestDockerProxy(t *testing.T) {
 		assert.Equal(t, "OK GET /v1.47/_ping", string(body))
 	})
 
+	t.Run("discovery caches only successes", func(t *testing.T) {
+		if runtime.GOOS != "linux" {
+			t.Skip("automatic proxy discovery requires Linux")
+		}
+		t.Setenv("DOCKER_HOST", "unix://"+daemonSocket)
+		t.Setenv("TMPDIR", shortTempDir(t))
+		require.Nil(t, NewDockerProxy(t.Context(), "failed"))
+		for _, job := range []string{"first", "second"} {
+			proxy := NewDockerProxy(t.Context(), job)
+			require.NotNil(t, proxy)
+			require.NoError(t, proxy.Close(context.Background()))
+		}
+		assert.EqualValues(t, 2, probes.Load())
+		dockerProxyDirectories.Range(func(key, value any) bool {
+			directory, _ := value.(dockerProxyDirectory)
+			directory.socket, _ = os.Stat(t.TempDir())
+			dockerProxyDirectories.Store(key, directory)
+			return true
+		})
+		proxy := NewDockerProxy(t.Context(), "replaced daemon")
+		require.NotNil(t, proxy)
+		require.NoError(t, proxy.Close(context.Background()))
+		assert.EqualValues(t, 3, probes.Load())
+	})
+
 	t.Run("tunnels raw streams through stdin EOF", func(t *testing.T) {
 		for _, stream := range []struct{ path, upgrade string }{
 			{path: "/v1.47/exec/abc/start"},

act/container/docker_run.go

@@ -50,15 +50,12 @@ const drainGracePeriod = 2 * time.Second
 
 // NewContainer creates a reference to a container
 func NewContainer(input *NewContainerInput) ExecutionsEnvironment {
-	cr := new(containerReference)
-	cr.input = input
 	// Resolved up front because the image pull runs before the container is created.
 	cf := createFlagsFromOptions(input.allOptions())
 	if cf.platform != "" {
-		cr.input.Platform = cf.platform
+		input.Platform = cf.platform
 	}
-	cr.pullPolicy = cf.pull
-	return cr
+	return &containerReference{input: input, pullPolicy: cf.pull}
 }
 
 // supportsContainerImagePlatform reports whether the Docker server API version
@@ -394,17 +391,16 @@ func GetDockerClient(ctx context.Context) (cli client.APIClient, err error) {
 	return cli, nil
 }
 
-func GetHostInfo(ctx context.Context) (info system.Info, err error) {
-	var cli client.APIClient
-	cli, err = GetDockerClient(ctx)
+func GetHostInfo(ctx context.Context) (system.Info, error) {
+	cli, err := GetDockerClient(ctx)
 	if err != nil {
-		return info, err
+		return system.Info{}, err
 	}
 	defer cli.Close()
 
 	result, err := cli.Info(ctx, client.InfoOptions{})
 	if err != nil {
-		return info, err
+		return system.Info{}, err
 	}
 
 	return result.Info, nil
@@ -476,22 +472,23 @@ func (cr *containerReference) find() common.Executor {
 			}
 			cr.id = ""
 		}
-		containers, err := cr.cli.ContainerList(ctx, client.ContainerListOptions{
-			All: true,
-		})
+		result, err := cr.cli.ContainerInspect(ctx, cr.input.Name, client.ContainerInspectOptions{})
+		if err == nil && result.Container.Name == "/"+cr.input.Name {
+			cr.id = result.Container.ID
+			return nil
+		} else if cerrdefs.IsNotFound(err) {
+			return nil
+		}
+		containers, err := cr.cli.ContainerList(ctx, client.ContainerListOptions{All: true}) // inspect also matches ids, and podman fails it with a 500 for a container removed mid-request
 		if err != nil {
 			return fmt.Errorf("failed to list containers: %w", err)
 		}
-
-		for _, c := range containers.Items {
-			for _, name := range c.Names {
-				if name[1:] == cr.input.Name {
-					cr.id = c.ID
-					return nil
+		for _, candidate := range containers.Items {
+			if slices.Contains(candidate.Names, "/"+cr.input.Name) {
+				cr.id = candidate.ID
+				break
 			}
 		}
-		}
-
 		return nil
 	}
 }
@@ -558,32 +555,32 @@ func (input *NewContainerInput) allOptions() string {
 	return strings.TrimSpace(input.RunnerOptions + " " + input.WorkflowOptions)
 }
 
-func (cr *containerReference) mergeContainerConfigs(ctx context.Context, config *container.Config, hostConfig *container.HostConfig) (*container.Config, *container.HostConfig, error) {
+func (cr *containerReference) mergeContainerConfigs(ctx context.Context, config *container.Config, hostConfig *container.HostConfig) error {
 	logger := common.Logger(ctx)
 	options := cr.input.allOptions()
 
 	if options == "" {
-		return config, hostConfig, nil
+		return nil
 	}
 
 	// For Gitea, checked here because the parse below is what would read those files
 	if err := rejectHostReadingOptions(cr.input.WorkflowOptions); err != nil {
-		return nil, nil, err
+		return err
 	}
 
 	// parse configuration from CLI container.options
 	flags, copts, cf, err := parseContainerOptions(options)
 	if err != nil {
-		return nil, nil, err
+		return err
 	}
 
 	if err := cf.validate(); err != nil {
-		return nil, nil, fmt.Errorf("cannot process container options: '%s': '%w'", options, err)
+		return fmt.Errorf("cannot process container options: '%s': '%w'", options, err)
 	}
 
 	if len(copts.netMode.Value()) == 0 {
 		if err = copts.netMode.Set(cr.input.NetworkMode); err != nil {
-			return nil, nil, fmt.Errorf("cannot parse networkmode=%s. This is an internal error and should not happen: '%w'", cr.input.NetworkMode, err)
+			return fmt.Errorf("cannot parse networkmode=%s. This is an internal error and should not happen: '%w'", cr.input.NetworkMode, err)
 		}
 	}
 
@@ -596,7 +593,7 @@ func (cr *containerReference) mergeContainerConfigs(ctx context.Context, config
 
 	containerConfig, err := parse(flags, copts, runtime.GOOS)
 	if err != nil {
-		return nil, nil, fmt.Errorf("cannot process container options: '%s': '%w'", options, err)
+		return fmt.Errorf("cannot process container options: '%s': '%w'", options, err)
 	}
 	// workflow aliases join the runner's own, deduped so a re-create cannot grow the input
 	for _, endpoint := range containerConfig.NetworkingConfig.EndpointsConfig {
@@ -611,7 +608,7 @@ func (cr *containerReference) mergeContainerConfigs(ctx context.Context, config
 	if !hostConfig.Privileged {
 		trusted, err := parseOptionsHostConfig(cr.input.RunnerOptions)
 		if err != nil {
-			return nil, nil, err
+			return err
 		}
 		sanitizeOptionsHostConfig(logger, containerConfig.HostConfig, trusted)
 	}
@@ -623,7 +620,7 @@ func (cr *containerReference) mergeContainerConfigs(ctx context.Context, config
 
 	logger.Debugf("Custom container.HostConfig from options ==> %+v", containerConfig.HostConfig)
 
-	cr.sanitizeConfig(ctx, nil, containerConfig.HostConfig) // a dropped volume must not displace the runner's own
+	cr.sanitizeConfig(ctx, containerConfig.HostConfig) // a dropped volume must not displace the runner's own
 	overlayVolumes(hostConfig, containerConfig.HostConfig)
 	binds := hostConfig.Binds
 	mounts := hostConfig.Mounts
@@ -644,7 +641,7 @@ func (cr *containerReference) mergeContainerConfigs(ctx context.Context, config
 	hostConfig.NetworkMode = networkMode
 	logger.Debugf("Merged container.HostConfig ==> %+v", hostConfig)
 
-	return config, hostConfig, nil
+	return nil
 }
 
 func (cr *containerReference) create(capAdd, capDrop []string) common.Executor {
@@ -710,8 +707,7 @@ func (cr *containerReference) create(capAdd, capDrop []string) common.Executor {
 			AutoRemove:   input.AutoRemove,
 		}
 
-		config, hostConfig, err = cr.mergeContainerConfigs(ctx, config, hostConfig)
-		if err != nil {
+		if err := cr.mergeContainerConfigs(ctx, config, hostConfig); err != nil {
 			return err
 		}
 
@@ -720,18 +716,15 @@ func (cr *containerReference) create(capAdd, capDrop []string) common.Executor {
 		}
 
 		// For Gitea
-		config, hostConfig = cr.sanitizeConfig(ctx, config, hostConfig)
+		cr.sanitizeConfig(ctx, hostConfig)
 
 		var networkingConfig *network.NetworkingConfig
 		n := hostConfig.NetworkMode
 		// IsUserDefined and IsHost are broken on windows
 		if n.IsUserDefined() && n != "host" && len(input.NetworkAliases) > 0 {
-			endpointConfig := &network.EndpointSettings{
-				Aliases: input.NetworkAliases,
-			}
 			networkingConfig = &network.NetworkingConfig{
 				EndpointsConfig: map[string]*network.EndpointSettings{
-					input.NetworkMode: endpointConfig,
+					input.NetworkMode: {Aliases: input.NetworkAliases},
 				},
 			}
 		}
@@ -772,16 +765,12 @@ func (cr *containerReference) exec(cmd []string, env map[string]string, user, wo
 
 		logger.Debugf("Exec command '%s'", cmd)
 		isTerminal := cr.input.AllocatePTY
-		var wd string
-		if workdir != "" {
+		wd := cr.input.WorkingDir
 		if strings.HasPrefix(workdir, "/") {
 			wd = workdir
-			} else {
+		} else if workdir != "" {
 			wd = fmt.Sprintf("%s/%s", cr.input.WorkingDir, workdir)
 		}
-		} else {
-			wd = cr.input.WorkingDir
-		}
 		logger.Debugf("Working directory '%s'", wd)
 
 		idResp, err := cr.cli.ExecCreate(ctx, cr.id, client.ExecCreateOptions{
@@ -805,8 +794,7 @@ func (cr *containerReference) exec(cmd []string, env map[string]string, user, wo
 		}
 		defer resp.Close()
 
-		err = cr.waitForCommand(ctx, resp.HijackedResponse)
-		if err != nil {
+		if err := cr.waitForCommand(ctx, resp.HijackedResponse); err != nil {
 			return err
 		}
 
@@ -908,17 +896,14 @@ func (cr *containerReference) copyDir(dstPath, srcPath string, useGitIgnore, ski
 			return err
 		}
 		logger.Debugf("Writing tarball %s from %s", tarFile.Name(), srcPath)
-		defer func(tarFile *os.File) {
-			name := tarFile.Name()
-			err := tarFile.Close()
-			if !errors.Is(err, os.ErrClosed) {
+		defer func() {
+			if err := tarFile.Close(); !errors.Is(err, os.ErrClosed) {
 				logger.Error(err)
 			}
-			err = os.Remove(name)
-			if err != nil {
+			if err := os.Remove(tarFile.Name()); err != nil {
 				logger.Error(err)
 			}
-		}(tarFile)
+		}()
 		extractionRoot := "/"
 		if actPath := cr.GetActPath(); strings.HasPrefix(dstPath+"/", actPath+"/") {
 			extractionRoot = actPath // keeps tar entries from crossing the image's /var/run symlink
@@ -928,8 +913,7 @@ func (cr *containerReference) copyDir(dstPath, srcPath string, useGitIgnore, ski
 		}
 
 		logger.Debugf("Extracting content from '%s' to '%s'", tarFile.Name(), dstPath)
-		_, err = tarFile.Seek(0, 0)
-		if err != nil {
+		if _, err := tarFile.Seek(0, 0); err != nil {
 			return fmt.Errorf("failed to seek tar archive: %w", err)
 		}
 		return cr.upload(ctx, extractionRoot, tarFile)
@@ -1097,9 +1081,7 @@ func parseOptionsHostConfig(options string) (*container.HostConfig, error) {
 	return containerConfig.HostConfig, nil
 }
 
-// For Gitea
-// sanitizeConfig remove the invalid configurations from `config` and `hostConfig`
-func (cr *containerReference) sanitizeConfig(ctx context.Context, config *container.Config, hostConfig *container.HostConfig) (*container.Config, *container.HostConfig) {
+func (cr *containerReference) sanitizeConfig(ctx context.Context, hostConfig *container.HostConfig) {
 	logger := common.Logger(ctx)
 	matcher := newValidVolumeMatcher(ctx, cr.input.ValidVolumes)
 
@@ -1148,13 +1130,12 @@ func (cr *containerReference) sanitizeConfig(ctx context.Context, config *contai
 		}
 	}
 	hostConfig.Mounts = sanitizedMounts
-
-	return config, hostConfig
 }
 
 func SanitizeBinds(ctx context.Context, validVolumes, binds []string) []string {
 	cr := &containerReference{input: &NewContainerInput{ValidVolumes: validVolumes}}
-	_, hostConfig := cr.sanitizeConfig(ctx, nil, &container.HostConfig{Binds: binds})
+	hostConfig := &container.HostConfig{Binds: binds}
+	cr.sanitizeConfig(ctx, hostConfig)
 	return hostConfig.Binds
 }
 

act/container/docker_run_test.go

@@ -13,9 +13,13 @@ import (
 	"errors"
 	"io"
 	"net"
+	"net/http"
+	"net/http/httptest"
 	"os"
 	"path/filepath"
 	"strings"
+	"sync"
+	"sync/atomic"
 	"testing"
 
 	"gitea.com/gitea/runner/act/common"
@@ -184,9 +188,7 @@ func TestDockerExecAbort(t *testing.T) {
 	cr := &containerReference{
 		id:    "123",
 		cli:   client,
-		input: &NewContainerInput{
-			Image: "image",
-		},
+		input: &NewContainerInput{},
 	}
 
 	channel := make(chan error)
@@ -209,12 +211,10 @@ func TestDockerExecAbort(t *testing.T) {
 func TestDockerExecFailure(t *testing.T) {
 	ctx := t.Context()
 
-	conn := &mockConn{}
-
 	client := &mockDockerClient{}
 	client.On("ExecCreate", ctx, "123", mock.AnythingOfType("client.ExecCreateOptions")).Return(mobyclient.ExecCreateResult{ID: "id"}, nil)
 	client.On("ExecAttach", ctx, "id", mock.AnythingOfType("client.ExecAttachOptions")).Return(mobyclient.ExecAttachResult{
-		Conn:   conn,
+		Conn:   &mockConn{},
 		Reader: bufio.NewReader(strings.NewReader("output")),
 	}, nil)
 	client.On("ExecInspect", ctx, "id", mobyclient.ExecInspectOptions{}).Return(mobyclient.ExecInspectResult{
@@ -224,9 +224,7 @@ func TestDockerExecFailure(t *testing.T) {
 	cr := &containerReference{
 		id:    "123",
 		cli:   client,
-		input: &NewContainerInput{
-			Image: "image",
-		},
+		input: &NewContainerInput{},
 	}
 
 	err := cr.exec([]string{""}, map[string]string{}, "user", "workdir")(ctx)
@@ -235,7 +233,6 @@ func TestDockerExecFailure(t *testing.T) {
 	assert.Equal(t, ExitCodeError(1), exitErr)
 	assert.Equal(t, "Process completed with exit code 1.", err.Error())
 
-	conn.AssertExpectations(t)
 	client.AssertExpectations(t)
 }
 
@@ -250,9 +247,6 @@ func stdcopyFrame(stream stdcopy.StdType, payload string) []byte {
 	return b
 }
 
-// TestDockerAttachFlushesTrailingLine verifies that wait() blocks until the
-// attach() streaming goroutine has drained and flushed the container's output,
-// so a final line without a trailing newline is not lost.
 func TestDockerAttachFlushesTrailingLine(t *testing.T) {
 	ctx := t.Context()
 
@@ -273,18 +267,13 @@ func TestDockerAttachFlushesTrailingLine(t *testing.T) {
 
 	statusCh := make(chan container.WaitResponse, 1)
 	statusCh <- container.WaitResponse{StatusCode: 0}
-	errCh := make(chan error, 1)
 	client.On("ContainerWait", ctx, "123", mobyclient.ContainerWaitOptions{Condition: container.WaitConditionNotRunning}).
-		Return(mobyclient.ContainerWaitResult{
-			Result: (<-chan container.WaitResponse)(statusCh),
-			Error:  (<-chan error)(errCh),
-		})
+		Return(mobyclient.ContainerWaitResult{Result: statusCh})
 
 	cr := &containerReference{
 		id:  "123",
 		cli: client,
 		input: &NewContainerInput{
-			Image:  "image",
 			Stdout: logWriter,
 			Stderr: logWriter,
 		},
@@ -293,10 +282,6 @@ func TestDockerAttachFlushesTrailingLine(t *testing.T) {
 	require.NoError(t, cr.attach()(ctx))
 	require.NoError(t, cr.wait()(ctx))
 
-	// wait() must have blocked until the goroutine drained AND flushed; the
-	// trailing, non-newline-terminated line must therefore be present. Reading
-	// lines here is race-free because wait() synchronizes on attachDone, which
-	// the goroutine closes after the final append.
 	assert.Equal(t, []string{"line one\n", "last line without newline"}, lines)
 
 	client.AssertExpectations(t)
@@ -307,24 +292,11 @@ func TestDockerWaitFailure(t *testing.T) {
 
 	statusCh := make(chan container.WaitResponse, 1)
 	statusCh <- container.WaitResponse{StatusCode: 2}
-	errCh := make(chan error, 1)
-
 	client := &mockDockerClient{}
 	client.On("ContainerWait", ctx, "123", mobyclient.ContainerWaitOptions{Condition: container.WaitConditionNotRunning}).
-		Return(mobyclient.ContainerWaitResult{
-			Result: (<-chan container.WaitResponse)(statusCh),
-			Error:  (<-chan error)(errCh),
-		})
+		Return(mobyclient.ContainerWaitResult{Result: statusCh})
 
-	cr := &containerReference{
-		id:  "123",
-		cli: client,
-		input: &NewContainerInput{
-			Image: "image",
-		},
-	}
-
-	err := cr.wait()(ctx)
+	err := (&containerReference{id: "123", cli: client}).wait()(ctx)
 	var exitErr ExitCodeError
 	require.ErrorAs(t, err, &exitErr)
 	assert.Equal(t, ExitCodeError(2), exitErr)
@@ -398,70 +370,81 @@ func TestRemoveWithoutIDUsesName(t *testing.T) {
 	client.AssertExpectations(t)
 }
 
-// find() must drop a stale cached id so later Copy/Exec don't hit the
-// daemon with a torn-down container.
 func TestFindRevalidatesStaleID(t *testing.T) {
 	ctx := t.Context()
+	inspectOpts := mobyclient.ContainerInspectOptions{}
 	notFound := cerrdefs.ErrNotFound.WithMessage("No such container")
 	boom := errors.New("daemon unreachable")
-	newCR := func(id string) (*containerReference, *mockDockerClient) {
+	for _, tc := range []struct {
+		name, cachedID string
+		cachedErr      error
+		lookup         container.InspectResponse
+		lookupErr      error
+		listed         []container.Summary
+		wantID         string
+		retryList      bool
+	}{
+		{name: "stale id cleared, name lookup empty", cachedID: "stale", cachedErr: notFound, lookupErr: notFound},
+		{name: "stale id cleared, name lookup repopulates", cachedID: "stale", cachedErr: notFound, lookup: container.InspectResponse{ID: "fresh", Name: "/job-1"}, wantID: "fresh"},
+		{name: "id match falls back to listing", lookup: container.InspectResponse{ID: "job-1abc", Name: "/somebody-else"}, listed: []container.Summary{{ID: "fresh", Names: []string{"/job-1"}}}, wantID: "fresh"},
+		{name: "live id kept", cachedID: "live", wantID: "live"},
+		{name: "transient inspect error trusts cache", cachedID: "live", cachedErr: boom, wantID: "live"},
+		{name: "name lookup error falls back to listing", lookupErr: boom, listed: []container.Summary{{ID: "other", Names: []string{"/somebody-else"}}, {ID: "fresh", Names: []string{"/job-1"}}}, wantID: "fresh", retryList: true},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
 			client := &mockDockerClient{}
-		return &containerReference{id: id, cli: client, input: &NewContainerInput{Name: "job-1"}}, client
+			cr := &containerReference{id: tc.cachedID, cli: client, input: &NewContainerInput{Name: "job-1"}}
+			if tc.cachedID != "" {
+				client.On("ContainerInspect", ctx, tc.cachedID, inspectOpts).Return(mobyclient.ContainerInspectResult{}, tc.cachedErr)
+			}
+			if tc.cachedID == "" || cerrdefs.IsNotFound(tc.cachedErr) {
+				client.On("ContainerInspect", ctx, "job-1", inspectOpts).Return(mobyclient.ContainerInspectResult{Container: tc.lookup}, tc.lookupErr)
+			}
+			if tc.listed != nil {
+				client.On("ContainerList", ctx, mobyclient.ContainerListOptions{All: true}).Return(mobyclient.ContainerListResult{Items: tc.listed}, nil).Once()
 			}
-	listOpts := mobyclient.ContainerListOptions{All: true}
-	inspectOpts := mobyclient.ContainerInspectOptions{}
-
-	t.Run("stale id cleared, name lookup empty", func(t *testing.T) {
-		cr, client := newCR("stale")
-		client.On("ContainerInspect", ctx, "stale", inspectOpts).Return(mobyclient.ContainerInspectResult{}, notFound)
-		client.On("ContainerList", ctx, listOpts).Return(mobyclient.ContainerListResult{}, nil)
 			require.NoError(t, cr.find()(ctx))
-		assert.Empty(t, cr.id)
-		client.AssertExpectations(t)
-	})
-
-	t.Run("stale id cleared, name lookup repopulates", func(t *testing.T) {
-		cr, client := newCR("stale")
-		client.On("ContainerInspect", ctx, "stale", inspectOpts).Return(mobyclient.ContainerInspectResult{}, notFound)
-		client.On("ContainerList", ctx, listOpts).Return(mobyclient.ContainerListResult{Items: []container.Summary{
-			{ID: "other", Names: []string{"/somebody-else"}},
-			{ID: "fresh", Names: []string{"/job-1"}},
-		}}, nil)
-		require.NoError(t, cr.find()(ctx))
-		assert.Equal(t, "fresh", cr.id)
-		client.AssertExpectations(t)
-	})
-
-	t.Run("live id kept", func(t *testing.T) {
-		cr, client := newCR("live")
-		client.On("ContainerInspect", ctx, "live", inspectOpts).Return(mobyclient.ContainerInspectResult{}, nil)
-		require.NoError(t, cr.find()(ctx))
-		assert.Equal(t, "live", cr.id)
-		client.AssertExpectations(t)
-	})
-
-	t.Run("transient inspect error trusts cache", func(t *testing.T) {
-		cr, client := newCR("live")
-		client.On("ContainerInspect", ctx, "live", inspectOpts).Return(mobyclient.ContainerInspectResult{}, boom)
-		require.NoError(t, cr.find()(ctx))
-		assert.Equal(t, "live", cr.id)
-		client.AssertExpectations(t)
-	})
-
-	t.Run("list error propagates", func(t *testing.T) {
-		cr, client := newCR("")
-		client.On("ContainerList", ctx, listOpts).Return(mobyclient.ContainerListResult{}, boom)
+			assert.Equal(t, tc.wantID, cr.id)
+			if tc.retryList {
+				client.On("ContainerList", ctx, mobyclient.ContainerListOptions{All: true}).Return(mobyclient.ContainerListResult{}, boom)
+				cr.id = ""
 				require.ErrorIs(t, cr.find()(ctx), boom)
+			}
 			client.AssertExpectations(t)
 		})
 	}
+}
+
+func TestRunnerArchCachedPerRun(t *testing.T) {
+	var infos atomic.Int32
+	daemon := httptest.NewServer(http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) {
+		writer.Header().Set("Content-Type", "application/json")
+		writer.Header().Set("API-Version", "1.47")
+		if strings.HasSuffix(request.URL.Path, "/info") {
+			infos.Add(1)
+			_, _ = io.WriteString(writer, `{"Architecture":"aarch64"}`)
+		}
+	}))
+	t.Cleanup(daemon.Close)
+	t.Setenv("DOCKER_HOST", daemon.URL)
+	extensions := &LinuxContainerEnvironmentExtensions{}
+	run := WithRunnerArch(t.Context())
+	var lookups sync.WaitGroup
+	for range 4 {
+		lookups.Go(func() { assert.Equal(t, "ARM64", extensions.GetRunnerContext(run)["arch"]) })
+	}
+	lookups.Wait()
+	assert.Equal(t, "ARM64", extensions.GetRunnerContext(t.Context())["arch"])
+	assert.EqualValues(t, 2, infos.Load())
+}
 
 // Every daemon entry point fails fast with a clear, container-named
 // error when no live cr.id is known.
 func TestRejectsMissingContainer(t *testing.T) {
 	ctx := t.Context()
 	client := &mockDockerClient{}
-	client.On("ContainerList", ctx, mobyclient.ContainerListOptions{All: true}).Return(mobyclient.ContainerListResult{}, nil)
+	client.On("ContainerInspect", ctx, "job-1", mobyclient.ContainerInspectOptions{}).
+		Return(mobyclient.ContainerInspectResult{}, cerrdefs.ErrNotFound)
 	cr := &containerReference{cli: client, input: &NewContainerInput{Name: "job-1"}}
 	check := func(op string, err error) {
 		t.Helper()
@@ -498,10 +481,8 @@ func TestPublicCopyPipelineHandlesStaleID(t *testing.T) {
 			client := &mockDockerClient{}
 			client.On("ContainerInspect", ctx, "stale", mobyclient.ContainerInspectOptions{}).
 				Return(mobyclient.ContainerInspectResult{}, cerrdefs.ErrNotFound.WithMessage("gone"))
-			client.On("ContainerList", ctx, mobyclient.ContainerListOptions{All: true}).
-				Return(mobyclient.ContainerListResult{Items: []container.Summary{
-					{ID: "fresh", Names: []string{"/job-1"}},
-				}}, nil)
+			client.On("ContainerInspect", ctx, "job-1", mobyclient.ContainerInspectOptions{}).
+				Return(mobyclient.ContainerInspectResult{Container: container.InspectResponse{ID: "fresh", Name: "/job-1"}}, nil)
 			client.On("CopyToContainer", ctx, "fresh", mock.Anything).
 				Run(func(args mock.Arguments) {
 					opts := args.Get(2).(mobyclient.CopyToContainerOptions)
@@ -525,9 +506,6 @@ func TestPublicCopyPipelineHandlesStaleID(t *testing.T) {
 	}
 }
 
-// Type assert containerReference implements ExecutionsEnvironment
-var _ ExecutionsEnvironment = &containerReference{}
-
 func TestCheckVolumes(t *testing.T) {
 	testCases := []struct {
 		desc           string
@@ -622,10 +600,11 @@ func TestCheckVolumes(t *testing.T) {
 					ValidVolumes: tc.validVolumes,
 				},
 			}
-			_, hostConf := cr.sanitizeConfig(ctx, &container.Config{}, &container.HostConfig{Binds: tc.binds, Mounts: tc.mounts})
+			hostConf := &container.HostConfig{Binds: tc.binds, Mounts: tc.mounts}
+			cr.sanitizeConfig(ctx, hostConf)
 			assert.Equal(t, tc.expectedBinds, hostConf.Binds)
 			assert.ElementsMatch(t, tc.expectedMounts, hostConf.Mounts)
-			assert.Len(t, hook.AllRecords(), len(tc.binds)+len(tc.mounts)-len(tc.expectedBinds)-len(tc.expectedMounts)) // every drop is warned about
+			assert.Len(t, hook.AllRecords(), len(tc.binds)+len(tc.mounts)-len(tc.expectedBinds)-len(tc.expectedMounts))
 		})
 	}
 }
@@ -645,8 +624,6 @@ func TestMergeContainerConfigsDropsVolumeDriversFromWorkflows(t *testing.T) {
 	assert.Equal(t, "job-escape", hostConfig.Mounts[0].Source)
 }
 
-// Both of these are read here, on the runner, so a workflow could read the runner's files
-// and environment with them.
 func TestMergeContainerConfigsKeepsTheRunnersFilesAndEnvToItself(t *testing.T) {
 	hostFile := filepath.Join(t.TempDir(), "host.env")
 	require.NoError(t, os.WriteFile(hostFile, []byte("STOLEN=from-the-host\n"), 0o600))
@@ -656,16 +633,12 @@ func TestMergeContainerConfigsKeepsTheRunnersFilesAndEnvToItself(t *testing.T) {
 		logger := logging.New(logtest.New())
 		cr := &containerReference{input: &NewContainerInput{NetworkMode: "bridge", WorkflowOptions: option}}
 
-		_, _, err := cr.mergeContainerConfigs(common.WithLogger(t.Context(), logger), &container.Config{}, &container.HostConfig{})
-		require.ErrorContains(t, err, "not allowed in a workflow")
+		require.ErrorContains(t, cr.mergeContainerConfigs(common.WithLogger(t.Context(), logger), &container.Config{}, &container.HostConfig{}), "not allowed in a workflow")
 
-		// the runner reading its own files is what those options are for
 		cr = &containerReference{input: &NewContainerInput{NetworkMode: "bridge", RunnerOptions: option}}
-		_, _, err = cr.mergeContainerConfigs(common.WithLogger(t.Context(), logger), &container.Config{}, &container.HostConfig{})
-		require.NoError(t, err)
+		require.NoError(t, cr.mergeContainerConfigs(common.WithLogger(t.Context(), logger), &container.Config{}, &container.HostConfig{}))
 	}
 
-	// a bare name is no longer resolved from the runner's environment, for either source
 	logger := logging.New(logtest.New())
 	cr := &containerReference{input: &NewContainerInput{
 		NetworkMode:     "bridge",
@@ -673,8 +646,8 @@ func TestMergeContainerConfigsKeepsTheRunnersFilesAndEnvToItself(t *testing.T) {
 		WorkflowOptions: "--env RUNNER_SECRET --env GIVEN=value",
 	}}
 
-	config, _, err := cr.mergeContainerConfigs(common.WithLogger(t.Context(), logger), &container.Config{}, &container.HostConfig{})
-	require.NoError(t, err)
+	config := &container.Config{}
+	require.NoError(t, cr.mergeContainerConfigs(common.WithLogger(t.Context(), logger), config, &container.HostConfig{}))
 	assert.Equal(t, []string{"RUNNER_SECRET", "RUNNER_SECRET", "GIVEN=value"}, config.Env)
 }
 
@@ -711,7 +684,6 @@ func TestSanitizeOptionsHostConfig(t *testing.T) {
 	assert.Equal(t, &container.HostConfig{}, hostConfig)
 }
 
-// mergeOptions merges both option sources into a bare container, returning the result and its log.
 func mergeOptions(t *testing.T, runnerOptions, workflowOptions string, privileged bool) (*container.HostConfig, *logtest.Handler) {
 	t.Helper()
 	logger, hook := logtest.NewLogger()
@@ -723,12 +695,12 @@ func mergeOptions(t *testing.T, runnerOptions, workflowOptions string, privilege
 		ValidVolumes:    []string{"**"},
 	}}
 
-	_, hostConfig, err := cr.mergeContainerConfigs(common.WithLogger(t.Context(), logger), &container.Config{}, &container.HostConfig{
+	hostConfig := &container.HostConfig{
 		Privileged:  privileged,
 		UsernsMode:  container.UsernsMode("private"),
 		NetworkMode: container.NetworkMode("bridge"),
-	})
-	require.NoError(t, err)
+	}
+	require.NoError(t, cr.mergeContainerConfigs(common.WithLogger(t.Context(), logger), &container.Config{}, hostConfig))
 	return hostConfig, hook
 }
 
@@ -782,23 +754,26 @@ func TestCheckVolumesRejectsEscapingHostPaths(t *testing.T) {
 	}
 
 	escapingPath := allowed + string(filepath.Separator) + ".." + string(filepath.Separator) + "denied"
-	_, hostConf := cr.sanitizeConfig(ctx, &container.Config{}, &container.HostConfig{
+	hostConf := &container.HostConfig{
 		Binds: []string{escapingPath + ":/mnt"},
-	})
+	}
+	cr.sanitizeConfig(ctx, hostConf)
 	assert.Empty(t, hostConf.Binds)
 
 	linkPath := filepath.Join(allowed, "link")
 	if err := os.Symlink(denied, linkPath); err != nil {
 		t.Skipf("cannot create symlink: %v", err)
 	}
-	_, hostConf = cr.sanitizeConfig(ctx, &container.Config{}, &container.HostConfig{
+	hostConf = &container.HostConfig{
 		Binds: []string{linkPath + ":/mnt"},
-	})
+	}
+	cr.sanitizeConfig(ctx, hostConf)
 	assert.Empty(t, hostConf.Binds)
 
-	_, hostConf = cr.sanitizeConfig(ctx, &container.Config{}, &container.HostConfig{
+	hostConf = &container.HostConfig{
 		Binds: []string{filepath.Join(linkPath, "missing") + ":/mnt"},
-	})
+	}
+	cr.sanitizeConfig(ctx, hostConf)
 	assert.Empty(t, hostConf.Binds)
 }
 
@@ -808,11 +783,12 @@ func TestSanitizeConfigKeepsTmpfsMounts(t *testing.T) {
 
 	for _, validVolumes := range [][]string{nil, {"data"}, {"**:ro"}} {
 		logger, hook := logtest.NewLogger()
-		_, sanitized := (&containerReference{input: &NewContainerInput{ValidVolumes: validVolumes}}).sanitizeConfig(common.WithLogger(t.Context(), logger), &container.Config{}, &container.HostConfig{Mounts: []mount.Mount{
+		sanitized := &container.HostConfig{Mounts: []mount.Mount{
 			hostConfig.Mounts[0],
 			{Type: mount.TypeVolume, Target: "/anonymous"},
 			{Type: mount.TypeTmpfs, Source: "/etc", Target: "/etc-copy"},
-		}})
+		}}
+		(&containerReference{input: &NewContainerInput{ValidVolumes: validVolumes}}).sanitizeConfig(common.WithLogger(t.Context(), logger), sanitized)
 		assert.Equal(t, hostConfig.Mounts, sanitized.Mounts)
 		assert.Len(t, hook.AllRecords(), 2)
 	}
@@ -908,11 +884,11 @@ func TestMergeContainerConfigsVolumesReplaceRunnerMounts(t *testing.T) {
 		},
 	}
 
-	_, hostConf, err := cr.mergeContainerConfigs(ctx, &container.Config{}, &container.HostConfig{
+	hostConf := &container.HostConfig{
 		Binds:  []string{"/var/run/docker.sock:/var/run/docker.sock"},
 		Mounts: []mount.Mount{{Type: mount.TypeVolume, Source: "act-toolcache", Target: "/opt/hostedtoolcache"}},
-	})
-	require.NoError(t, err)
+	}
+	require.NoError(t, cr.mergeContainerConfigs(ctx, &container.Config{}, hostConf))
 	assert.Equal(t, []string{"/var/run/docker.sock:/var/run/docker.sock", "/host/tools:/opt/hostedtoolcache"}, hostConf.Binds)
 	assert.Empty(t, hostConf.Mounts)
 }
@@ -964,8 +940,7 @@ func TestMergeContainerConfigsKeepsNetworkAliasesFromOptions(t *testing.T) {
 		NetworkMode: "job-network", NetworkAliases: []string{"redis"}, WorkflowOptions: "--network-alias redis-primary",
 	}}
 
-	_, _, err := cr.mergeContainerConfigs(common.WithLogger(t.Context(), logger), &container.Config{}, &container.HostConfig{})
-	require.NoError(t, err)
+	require.NoError(t, cr.mergeContainerConfigs(common.WithLogger(t.Context(), logger), &container.Config{}, &container.HostConfig{}))
 	assert.Equal(t, []string{"redis", "redis-primary"}, cr.input.NetworkAliases)
 }
 

act/container/host_environment.go

@@ -7,6 +7,7 @@ package container
 import (
 	"archive/tar"
 	"bytes"
+	"cmp"
 	"context"
 	"errors"
 	"fmt"
@@ -64,15 +65,11 @@ func (e *HostEnvironment) processGroup(ctx context.Context) *process.Group {
 }
 
 func (e *HostEnvironment) Create(_, _ []string) common.Executor {
-	return func(ctx context.Context) error {
-		return nil
-	}
+	return common.NewPipelineExecutor()
 }
 
 func (e *HostEnvironment) Close() common.Executor {
-	return func(ctx context.Context) error {
-		return nil
-	}
+	return common.NewPipelineExecutor()
 }
 
 func (e *HostEnvironment) Copy(destPath string, files ...*FileEntry) common.Executor {
@@ -168,15 +165,11 @@ func (e *HostEnvironment) GetContainerArchive(ctx context.Context, srcPath strin
 }
 
 func (e *HostEnvironment) Pull(_ bool) common.Executor {
-	return func(ctx context.Context) error {
-		return nil
-	}
+	return common.NewPipelineExecutor()
 }
 
 func (e *HostEnvironment) Start(_ bool) common.Executor {
-	return func(ctx context.Context) error {
-		return nil
-	}
+	return common.NewPipelineExecutor()
 }
 
 type ptyWriter struct {
@@ -193,7 +186,6 @@ func (w *ptyWriter) Write(buf []byte) (int, error) {
 		}
 		if w.dirtyLine || len(buf) > 1 && buf[len(buf)-2] != '\n' {
 			_, _ = w.Out.Write([]byte("\n"))
-			return n, io.EOF
 		}
 		return n, io.EOF
 	}
@@ -242,40 +234,25 @@ func writeKeepAlive(ppty io.Writer) {
 	}
 }
 
-func copyPtyOutput(writer io.Writer, ppty io.Reader, finishLog context.CancelFunc) {
-	defer func() {
-		finishLog()
-	}()
-	if _, err := io.Copy(writer, ppty); err != nil {
-		return
-	}
-}
-
 func (e *HostEnvironment) UpdateFromImageEnv(_ *map[string]string) common.Executor {
-	return func(ctx context.Context) error {
-		return nil
-	}
+	return common.NewPipelineExecutor()
 }
 
 func getEnvListFromMap(env map[string]string) []string {
-	envList := make([]string, 0)
-	for k, v := range env {
-		envList = append(envList, fmt.Sprintf("%s=%s", k, v))
+	envList := make([]string, 0, len(env))
+	for key, value := range env {
+		envList = append(envList, key+"="+value)
 	}
 	return envList
 }
 
-func (e *HostEnvironment) exec(ctx context.Context, command []string, cmdline string, env map[string]string, _, workdir string) error {
-	envList := getEnvListFromMap(env)
-	var wd string
+func (e *HostEnvironment) exec(ctx context.Context, command []string, cmdline string, env map[string]string, workdir string) error {
+	wd := e.Path
 	if workdir != "" {
-		if filepath.IsAbs(workdir) {
 		wd = workdir
-		} else {
+		if !filepath.IsAbs(workdir) {
 			wd = filepath.Join(e.Path, workdir)
 		}
-	} else {
-		wd = e.Path
 	}
 	// Flush any buffered, not-yet-newline-terminated trailing line, as the docker backend
 	// does in waitForCommand, so the final line of a command's output is not lost.
@@ -288,9 +265,8 @@ func (e *HostEnvironment) exec(ctx context.Context, command []string, cmdline st
 	cmd := exec.CommandContext(ctx, f)
 	cmd.Path = f
 	cmd.Args = command
-	cmd.Stdin = nil
 	cmd.Stdout = e.StdOut
-	cmd.Env = envList
+	cmd.Env = getEnvListFromMap(env)
 	cmd.Stderr = e.StdOut
 	cmd.Dir = wd
 	cmd.SysProcAttr = process.SysProcAttr(cmdline, false)
@@ -322,7 +298,10 @@ func (e *HostEnvironment) exec(ctx context.Context, command []string, cmdline st
 		writer = &ptyWriter{Out: e.StdOut}
 		var finishLog context.CancelFunc
 		logctx, finishLog = context.WithCancel(context.Background())
-		go copyPtyOutput(writer, ppty, finishLog)
+		go func() {
+			defer finishLog()
+			_, _ = io.Copy(writer, ppty)
+		}()
 		go writeKeepAlive(ppty)
 	}
 	if err := cmd.Start(); err != nil {
@@ -357,13 +336,13 @@ func (e *HostEnvironment) exec(ctx context.Context, command []string, cmdline st
 	return err
 }
 
-func (e *HostEnvironment) Exec(command []string /*cmdline string, */, env map[string]string, user, workdir string) common.Executor {
-	return e.ExecWithCmdLine(command, "", env, user, workdir)
+func (e *HostEnvironment) Exec(command []string, env map[string]string, _user, workdir string) common.Executor {
+	return e.ExecWithCmdLine(command, "", env, workdir)
 }
 
-func (e *HostEnvironment) ExecWithCmdLine(command []string, cmdline string, env map[string]string, user, workdir string) common.Executor {
+func (e *HostEnvironment) ExecWithCmdLine(command []string, cmdline string, env map[string]string, workdir string) common.Executor {
 	return func(ctx context.Context) error {
-		if err := e.exec(ctx, command, cmdline, env, user, workdir); err != nil {
+		if err := e.exec(ctx, command, cmdline, env, workdir); err != nil {
 			select {
 			case <-ctx.Done():
 				return fmt.Errorf("this step has been cancelled: %w", err)
@@ -605,8 +584,7 @@ func (*HostEnvironment) GetPathVariableName() string {
 }
 
 func (e *HostEnvironment) DefaultPathVariable() string {
-	v, _ := os.LookupEnv(e.GetPathVariableName())
-	return v
+	return os.Getenv(e.GetPathVariableName())
 }
 
 func (*HostEnvironment) JoinPathVariable(paths ...string) string {
@@ -616,30 +594,22 @@ func (*HostEnvironment) JoinPathVariable(paths ...string) string {
 // Reference for Arch values for runner.arch
 // https://docs.github.com/en/actions/learn-github-actions/contexts#runner-context
 func goArchToActionArch(arch string) string {
-	archMapper := map[string]string{
+	return cmp.Or(map[string]string{
 		"x86_64":  "X64",
 		"amd64":   "X64",
 		"386":     "X86",
 		"arm":     "ARM",
 		"aarch64": "ARM64",
 		"arm64":   "ARM64",
-	}
-	if arch, ok := archMapper[arch]; ok {
-		return arch
-	}
-	return arch
+	}[arch], arch)
 }
 
 func goOsToActionOs(os string) string {
-	osMapper := map[string]string{
+	return cmp.Or(map[string]string{
 		"linux":   "Linux",
 		"darwin":  "macOS",
 		"windows": "Windows",
-	}
-	if os, ok := osMapper[os]; ok {
-		return os
-	}
-	return os
+	}[os], os)
 }
 
 func (e *HostEnvironment) GetRunnerContext(_ context.Context) map[string]any {

act/container/host_environment_test.go

@@ -10,7 +10,6 @@ import (
 	"context"
 	"io"
 	"os"
-	"path"
 	"path/filepath"
 	"runtime"
 	"strings"
@@ -24,7 +23,6 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-// Type assert HostEnvironment implements ExecutionsEnvironment
 var _ ExecutionsEnvironment = &HostEnvironment{}
 
 func TestActionPlatformNames(t *testing.T) {
@@ -61,32 +59,20 @@ func TestCopyDir(t *testing.T) {
 }
 
 func TestGetContainerArchive(t *testing.T) {
-	dir := t.TempDir()
 	ctx := t.Context()
-	e := &HostEnvironment{
-		Path:      filepath.Join(dir, "path"),
-		TmpDir:    filepath.Join(dir, "tmp"),
-		ToolCache: filepath.Join(dir, "tool_cache"),
-		ActPath:   filepath.Join(dir, "act_path"),
-		StdOut:    os.Stdout,
-		Workdir:   path.Join("testdata", "scratch"),
-	}
-	_ = os.MkdirAll(e.Path, 0o700)
-	_ = os.MkdirAll(e.TmpDir, 0o700)
-	_ = os.MkdirAll(e.ToolCache, 0o700)
-	_ = os.MkdirAll(e.ActPath, 0o700)
+	e := &HostEnvironment{Path: t.TempDir()}
 	expectedContent := []byte("sdde/7sh")
 	err := os.WriteFile(filepath.Join(e.Path, "action.yml"), expectedContent, 0o600)
-	assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act
+	require.NoError(t, err)
 	archive, err := e.GetContainerArchive(ctx, e.Path)
-	assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act
+	require.NoError(t, err)
 	defer archive.Close()
 	reader := tar.NewReader(archive)
 	h, err := reader.Next()
-	assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act
+	require.NoError(t, err)
 	assert.Equal(t, "action.yml", h.Name)
 	content, err := io.ReadAll(reader)
-	assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act
+	require.NoError(t, err)
 	assert.Equal(t, expectedContent, content)
 	_, err = reader.Next()
 	assert.ErrorIs(t, err, io.EOF)
@@ -96,19 +82,8 @@ func TestHostEnvironmentExecExitCode(t *testing.T) {
 	if runtime.GOOS == "windows" {
 		t.Skip("uses POSIX shell")
 	}
-	dir := t.TempDir()
 	ctx := t.Context()
-	e := &HostEnvironment{
-		Path:      filepath.Join(dir, "path"),
-		TmpDir:    filepath.Join(dir, "tmp"),
-		ToolCache: filepath.Join(dir, "tool_cache"),
-		ActPath:   filepath.Join(dir, "act_path"),
-		StdOut:    io.Discard,
-		Workdir:   filepath.Join(dir, "path"),
-	}
-	for _, p := range []string{e.Path, e.TmpDir, e.ToolCache, e.ActPath} {
-		assert.NoError(t, os.MkdirAll(p, 0o700)) //nolint:testifylint // test setup
-	}
+	e := &HostEnvironment{Path: t.TempDir(), StdOut: io.Discard}
 
 	err := e.Exec([]string{"sh", "-c", "exit 3"}, map[string]string{"PATH": os.Getenv("PATH")}, "", "")(ctx)
 	var exitErr ExitCodeError
@@ -130,20 +105,12 @@ func TestHostEnvironmentAllocatePTY(t *testing.T) {
 		{name: "on", allocPTY: true, expect: "TTY"},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
-			dir := t.TempDir()
 			buf := &bytes.Buffer{}
 			e := &HostEnvironment{
-				Path:        filepath.Join(dir, "path"),
-				TmpDir:      filepath.Join(dir, "tmp"),
-				ToolCache:   filepath.Join(dir, "tool_cache"),
-				ActPath:     filepath.Join(dir, "act_path"),
+				Path:        t.TempDir(),
 				StdOut:      buf,
-				Workdir:     filepath.Join(dir, "path"),
 				AllocatePTY: tc.allocPTY,
 			}
-			for _, p := range []string{e.Path, e.TmpDir, e.ToolCache, e.ActPath} {
-				require.NoError(t, os.MkdirAll(p, 0o700))
-			}
 
 			err := e.Exec(
 				[]string{"sh", "-c", "[ -t 1 ] && printf TTY || printf NOTTY"},
@@ -156,49 +123,26 @@ func TestHostEnvironmentAllocatePTY(t *testing.T) {
 	}
 }
 
-func TestHostEnvironmentRemovePreservesWorkdirByDefault(t *testing.T) {
-	ctx := common.WithLogger(t.Context(), logging.Default())
-	base := t.TempDir()
-	miscRoot := filepath.Join(base, "misc")
-	path := filepath.Join(miscRoot, "hostexecutor")
-	require.NoError(t, os.MkdirAll(path, 0o700))
-	workdir := filepath.Join(base, "workspace", "owner", "repo")
-	require.NoError(t, os.MkdirAll(workdir, 0o700))
-
+func TestHostEnvironmentRemoveWorkdir(t *testing.T) {
+	for name, owned := range map[string]bool{"preserves caller workdir": false, "cleans owned workdir": true} {
+		t.Run(name, func(t *testing.T) {
+			miscRoot, workdir := t.TempDir(), t.TempDir()
 			e := &HostEnvironment{
-		Path:    path,
+				Path:         filepath.Join(miscRoot, "hostexecutor"),
 				Workdir:      workdir,
-		CleanUp: func() {
-			_ = os.RemoveAll(miscRoot)
-		},
-		StdOut: os.Stdout,
+				CleanWorkdir: owned,
+				CleanUp:      func() { _ = os.RemoveAll(miscRoot) },
 			}
-	require.NoError(t, e.Remove()(ctx))
+			require.NoError(t, os.MkdirAll(e.Path, 0o700))
+			require.NoError(t, e.Remove()(t.Context()))
 			_, err := os.Stat(workdir)
+			if owned {
+				assert.ErrorIs(t, err, os.ErrNotExist)
+			} else {
 				require.NoError(t, err)
 			}
-
-func TestHostEnvironmentRemoveCleansWorkdirWhenOwned(t *testing.T) {
-	ctx := common.WithLogger(t.Context(), logging.Default())
-	base := t.TempDir()
-	miscRoot := filepath.Join(base, "misc")
-	path := filepath.Join(miscRoot, "hostexecutor")
-	require.NoError(t, os.MkdirAll(path, 0o700))
-	workdir := filepath.Join(base, "workspace", "123", "owner", "repo")
-	require.NoError(t, os.MkdirAll(workdir, 0o700))
-
-	e := &HostEnvironment{
-		Path:         path,
-		Workdir:      workdir,
-		CleanWorkdir: true,
-		CleanUp: func() {
-			_ = os.RemoveAll(miscRoot)
-		},
-		StdOut: os.Stdout,
+		})
 	}
-	require.NoError(t, e.Remove()(ctx))
-	_, err := os.Stat(workdir)
-	assert.ErrorIs(t, err, os.ErrNotExist)
 }
 
 func TestRemoveAllWithContextDoesNotHangOnStuckDelete(t *testing.T) {
@@ -211,9 +155,6 @@ func TestRemoveAllWithContextDoesNotHangOnStuckDelete(t *testing.T) {
 		<-release
 		return nil
 	}
-	// removeAllWithContext intentionally leaks the delete goroutine on timeout,
-	// and that goroutine still references removeAll. Unblock it and wait for it
-	// to return before restoring the var, so the restore can't race the read.
 	t.Cleanup(func() {
 		close(release)
 		<-stubDone
@@ -227,87 +168,42 @@ func TestRemoveAllWithContextDoesNotHangOnStuckDelete(t *testing.T) {
 	require.ErrorIs(t, err, context.DeadlineExceeded)
 }
 
-// TestHostEnvironmentRemoveDoesNotHangOnStuckCleanUp guards against a stalled
-// CleanUp callback (e.g. an os.RemoveAll blocked by an AV/EDR filter driver or
-// an unresponsive mount) wedging the runner slot forever at "Cleaning up
-// container". Remove must time out the callback and complete job teardown.
-func TestHostEnvironmentRemoveDoesNotHangOnStuckCleanUp(t *testing.T) {
-	// Keep the suite fast: shrink the per-phase teardown timeout for this test.
-	orig := hostCleanupTimeout
-	hostCleanupTimeout = 100 * time.Millisecond
-	t.Cleanup(func() { hostCleanupTimeout = orig })
-
-	ctx := common.WithLogger(t.Context(), logging.Default())
-	base := t.TempDir()
-	path := filepath.Join(base, "misc", "hostexecutor")
-	require.NoError(t, os.MkdirAll(path, 0o700))
-
-	release := make(chan struct{})
-	t.Cleanup(func() { close(release) }) // unblock the leaked goroutine at test end
-
-	e := &HostEnvironment{
-		Path: path,
-		CleanUp: func() {
-			<-release // simulate a delete syscall stuck indefinitely
-		},
-		StdOut: os.Stdout,
-	}
-
-	done := make(chan error, 1)
-	go func() { done <- e.Remove()(ctx) }()
-
-	select {
-	case err := <-done:
-		require.NoError(t, err)
-	case <-time.After(10 * time.Second):
-		t.Fatal("Remove() hung on a stuck CleanUp callback")
-	}
-}
-
-// TestHostEnvironmentRemoveDoesNotHangOnStuckPathRemoval guards against a
-// stalled os.RemoveAll on the misc/workspace paths (same AV/EDR wedge as
-// #1023) wedging job completion after the CleanUp callback has already timed
-// out or finished.
-func TestHostEnvironmentRemoveDoesNotHangOnStuckPathRemoval(t *testing.T) {
+func TestHostEnvironmentRemoveDoesNotHang(t *testing.T) {
+	for _, phase := range []string{"CleanUp callback", "path removal"} {
+		t.Run(phase, func(t *testing.T) {
 			origTimeout := hostCleanupTimeout
 			hostCleanupTimeout = 100 * time.Millisecond
 			t.Cleanup(func() { hostCleanupTimeout = origTimeout })
-
-	release := make(chan struct{})
-	stubDone := make(chan struct{})
-
-	origRemoveAll := removeAll
-	removeAll = func(string) error {
+			release, stubDone := make(chan struct{}), make(chan struct{})
+			block := func() {
 				defer close(stubDone)
 				<-release
+			}
+			e := &HostEnvironment{Path: filepath.Join(t.TempDir(), "misc", "hostexecutor"), StdOut: os.Stdout}
+			if phase == "CleanUp callback" {
+				e.CleanUp = block
+			} else {
+				origRemoveAll := removeAll
+				removeAll = func(string) error {
+					block()
 					return nil
 				}
-	// The stuck delete goroutine outlives the timed-out Remove and still reads
-	// removeAll; unblock it and wait before restoring to avoid a restore/read race.
+				t.Cleanup(func() { removeAll = origRemoveAll })
+			}
 			t.Cleanup(func() {
 				close(release)
 				<-stubDone
-		removeAll = origRemoveAll
 			})
-
-	ctx := common.WithLogger(t.Context(), logging.Default())
-	base := t.TempDir()
-	path := filepath.Join(base, "misc", "hostexecutor")
-	require.NoError(t, os.MkdirAll(path, 0o700))
-
-	e := &HostEnvironment{
-		Path:   path,
-		StdOut: os.Stdout,
-	}
-
+			require.NoError(t, os.MkdirAll(e.Path, 0o700))
 			done := make(chan error, 1)
-	go func() { done <- e.Remove()(ctx) }()
-
+			go func() { done <- e.Remove()(common.WithLogger(t.Context(), logging.Default())) }()
 			select {
 			case err := <-done:
 				require.NoError(t, err)
 			case <-time.After(10 * time.Second):
-		t.Fatal("Remove() hung on a stuck path removal")
+				t.Fatalf("Remove() hung on a stuck %s", phase)
+			}
+		})
 	}
 }
 
@@ -315,17 +211,11 @@ func TestBuildWindowsWorkspaceKillScript(t *testing.T) {
 	t.Run("single dir", func(t *testing.T) {
 		s := buildWindowsWorkspaceKillScript([]string{`C:\workspace\job1`})
 		assert.Contains(t, s, `$paths = @('C:\workspace\job1')`)
-		// Self-PID guard is essential — without it the script could taskkill
-		// the PowerShell process running it.
 		assert.Contains(t, s, "$selfPid = $PID")
 		assert.Contains(t, s, "$_.ProcessId -eq $selfPid")
-		// Must match both ExecutablePath (binaries from the workspace) and
-		// CommandLine (system binaries invoked with workspace paths in args),
-		// both bounded by dir+separator so a name-prefix sibling is spared.
 		assert.Contains(t, s, `$prefix = $p + '\'`)
 		assert.Contains(t, s, "$_.ExecutablePath.StartsWith($prefix")
 		assert.Contains(t, s, "$_.CommandLine.IndexOf($prefix")
-		// Each matched PID must be tree-killed, not just stopped.
 		assert.Contains(t, s, "taskkill.exe /PID $_.ProcessId /T /F")
 	})
 
@@ -338,25 +228,16 @@ func TestBuildWindowsWorkspaceKillScript(t *testing.T) {
 		assert.Contains(t, s, `'C:\work\path'`)
 		assert.Contains(t, s, `'C:\work\workdir'`)
 		assert.Contains(t, s, `'C:\Users\runner\AppData\Local\Temp\job-42'`)
-		// Commas between entries — no trailing comma, no leading comma.
 		assert.Contains(t, s, `'C:\work\path','C:\work\workdir',`)
 	})
 
 	t.Run("path with single quote is escaped", func(t *testing.T) {
-		// In PowerShell single-quoted strings the only special char is the
-		// quote itself, escaped by doubling. A workspace path that ever
-		// contained `'` would inject a command into the script otherwise.
 		s := buildWindowsWorkspaceKillScript([]string{`C:\work\it's\path`})
 		assert.Contains(t, s, `'C:\work\it''s\path'`)
-		// And it must NOT appear unescaped — otherwise the quote would
-		// terminate the literal early.
 		assert.NotContains(t, s, `'C:\work\it's\path'`)
 	})
 
 	t.Run("path with wildcard metacharacters is matched literally", func(t *testing.T) {
-		// A path containing [ ] ? * must be embedded verbatim and matched with
-		// ordinal String methods, not -like, otherwise the metacharacters would
-		// be interpreted as wildcards and the leftover process could escape.
 		s := buildWindowsWorkspaceKillScript([]string{`C:\work\[job]?1`})
 		assert.Contains(t, s, `'C:\work\[job]?1'`)
 		assert.NotContains(t, s, "-like")
@@ -366,7 +247,6 @@ func TestBuildWindowsWorkspaceKillScript(t *testing.T) {
 
 	t.Run("empty dir list still produces a valid script", func(t *testing.T) {
 		s := buildWindowsWorkspaceKillScript(nil)
-		// Empty array literal — script runs, matches nothing, is a no-op.
 		assert.Contains(t, s, "$paths = @()")
 		assert.Contains(t, s, "Get-CimInstance Win32_Process")
 	})

act/container/kubernetes.go

@@ -387,16 +387,41 @@ func (c *kubernetesContainer) Start(_ bool) common.Executor {
 		if err := c.pod.waitStarted(ctx); err != nil {
 			return err
 		}
-		var stdout bytes.Buffer
-		if err := c.pod.client.exec(ctx, c.pod.name, c.name, []string{"sh", "-c", `printf '%s\n%s' "$(uname -m)" "$PATH"`}, nil, &stdout, io.Discard); err != nil {
+		output, err := c.pod.probe(ctx, c.name)
+		if err != nil {
 			return err
 		}
-		machine, path, _ := strings.Cut(stdout.String(), "\n")
+		machine, path, _ := strings.Cut(output, "\n")
 		c.pod.arch, c.pod.path = goArchToActionArch(machine), path
 		return nil
 	}
 }
 
+// probe retries exec while it fails short of the container, as a new node runs pods before its kubelet has a serving certificate.
+func (p *KubernetesPod) probe(ctx context.Context, container string) (string, error) {
+	var failure string
+	deadline := time.Now().Add(time.Minute)
+	for delay := 100 * time.Millisecond; ; delay = min(2*delay, 2*time.Second) {
+		var stdout bytes.Buffer
+		err := p.client.exec(ctx, p.name, container, []string{"sh", "-c", `printf '%s\n%s' "$(uname -m)" "$PATH"`}, nil, &stdout, io.Discard)
+		var exitErr ExitCodeError
+		var apiErr *apiError
+		if err == nil || ctx.Err() != nil || errors.As(err, &exitErr) || time.Now().After(deadline) ||
+			errors.As(err, &apiErr) && apiErr.code < http.StatusInternalServerError {
+			return stdout.String(), err
+		}
+		if err.Error() != failure {
+			failure = err.Error()
+			common.Logger(ctx).Infof("Waiting for exec into pod %s: %v", p.name, err)
+		}
+		select {
+		case <-ctx.Done():
+			return "", ctx.Err()
+		case <-time.After(delay):
+		}
+	}
+}
+
 func (*kubernetesContainer) Pull(bool) common.Executor {
 	return func(context.Context) error { return nil }
 }

act/container/kubernetes_test.go

@@ -45,6 +45,7 @@ type podManifest struct {
 type fakeCluster struct {
 	mu           sync.Mutex
 	requests     map[string][]string
+	execFailures []http.HandlerFunc
 }
 
 func (f *fakeCluster) record(request, content string) {
@@ -53,6 +54,41 @@ func (f *fakeCluster) record(request, content string) {
 	f.requests[request] = append(f.requests[request], content)
 }
 
+func (f *fakeCluster) failExec(failures ...http.HandlerFunc) {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+	f.execFailures = failures
+}
+
+func (f *fakeCluster) nextExecFailure() http.HandlerFunc {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+	if len(f.execFailures) == 0 {
+		return nil
+	}
+	failure := f.execFailures[0]
+	f.execFailures = f.execFailures[1:]
+	return failure
+}
+
+// execFailure answers like the API server, code 0 for a failure reported after the websocket upgrade.
+func execFailure(t *testing.T, code int, status string) http.HandlerFunc {
+	return func(w http.ResponseWriter, r *http.Request) {
+		if code != 0 {
+			w.WriteHeader(code)
+			_, _ = io.WriteString(w, status)
+			return
+		}
+		conn, err := websocket.Accept(w, r, &websocket.AcceptOptions{Subprotocols: []string{execProtocol}})
+		if err != nil {
+			t.Error(err)
+			return
+		}
+		defer func() { _ = conn.CloseNow() }()
+		_ = conn.Write(r.Context(), websocket.MessageBinary, append([]byte{3}, status...))
+	}
+}
+
 func fakeKubernetes(t *testing.T, podStatus string) (string, *fakeCluster) {
 	cluster := &fakeCluster{requests: map[string][]string{}}
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -70,7 +106,11 @@ func fakeKubernetes(t *testing.T, podStatus string) (string, *fakeCluster) {
 			_, _ = io.WriteString(w, `{"items":[{"metadata":{"name":"old","creationTimestamp":"2020-01-01T00:00:00Z"}},{"metadata":{"name":"new","creationTimestamp":"2999-01-01T00:00:00Z"}}]}`)
 		case strings.HasSuffix(r.URL.Path, "/exec"):
 			cluster.record("exec", r.URL.RawQuery)
+			if failure := cluster.nextExecFailure(); failure != nil {
+				failure(w, r)
+			} else {
 				serveLocalExec(t, w, r)
+			}
 		case strings.HasSuffix(r.URL.Path, "/log"):
 			_, _ = io.WriteString(w, "log of "+r.URL.Query().Get("container"))
 		case r.Method == http.MethodGet && strings.HasPrefix(r.URL.Path, pods+"/"):
@@ -213,7 +253,12 @@ func TestKubernetesPodRunsJobThroughAPI(t *testing.T) {
 
 	require.NoError(t, os.WriteFile(filepath.Join(workdir, "env"), []byte("#!/bin/sh\necho \"$@\" > \"$0-args\"\nexec /usr/bin/env \"$@\"\n"), 0o755))
 	t.Setenv("PATH", workdir+":"+os.Getenv("PATH"))
+	cluster.failExec(
+		execFailure(t, http.StatusInternalServerError, `{"message":"error dialing backend: remote error: tls: internal error"}`),
+		execFailure(t, 0, `{"status":"Failure","reason":"InternalError","message":"Internal error occurred: error sending request: remote error: tls: internal error"}`),
+	)
 	require.NoError(t, job.Start(false)(ctx))
+	assert.Len(t, cluster.requests["exec"], 3)
 
 	stepEnv := map[string]string{"INPUT_WHO-TO-GREET": "it's\nme", "SECRET": "hunter2", "UID": "5"}
 	require.NoError(t, job.Exec([]string{"printenv", "SECRET"}, stepEnv, "", "")(ctx))
@@ -295,6 +340,21 @@ func TestKubernetesPodFailsBeforeSteps(t *testing.T) {
 		}
 		require.ErrorContains(t, job.Create(nil, nil)(t.Context()), test.err)
 	}
+	running, cluster := fakeKubernetes(t, `{"spec":{"containers":[{}]},"status":{"containerStatuses":[{"name":"job","state":{"running":{}}}]}}`)
+	for _, test := range []struct {
+		failure http.HandlerFunc
+		err     string
+	}{
+		{execFailure(t, http.StatusForbidden, `{"message":"pods \"job\" is forbidden"}`), "is forbidden"},
+		{execFailure(t, 0, `{"status":"Failure","details":{"causes":[{"reason":"ExitCode","message":"126"}]}}`), ExitCodeError(126).Error()},
+	} {
+		cluster.failExec(test.failure)
+		pod, err := NewKubernetesPod(KubernetesOptions{Kubeconfig: running})
+		require.NoError(t, err)
+		job := pod.JobContainer(&NewContainerInput{Image: "node"})
+		require.NoError(t, job.Create(nil, nil)(t.Context()))
+		require.ErrorContains(t, job.Start(false)(t.Context()), test.err)
+	}
 	pod, err := NewKubernetesPod(KubernetesOptions{Kubeconfig: kubeconfig, Namespace: "ci"})
 	require.NoError(t, err)
 	job := pod.JobContainer(&NewContainerInput{Image: "missing"})

act/container/linux_container_environment_extensions.go

@@ -16,6 +16,8 @@ import (
 
 type LinuxContainerEnvironmentExtensions struct{}
 
+var windowsPathRegex = regexp.MustCompile(`^([a-zA-Z]):\\(.+)$`)
+
 // Resolves the equivalent host path inside the container
 // This is required for windows and WSL 2 to translate things like C:\Users\Myproject to /mnt/users/Myproject
 // For use in docker volumes and binds
@@ -31,8 +33,6 @@ func (*LinuxContainerEnvironmentExtensions) ToContainerPath(path string) string
 		return ""
 	}
 
-	// Test if the path is a windows path
-	windowsPathRegex := regexp.MustCompile(`^([a-zA-Z]):\\(.+)$`)
 	windowsPathComponents := windowsPathRegex.FindStringSubmatch(abspath)
 
 	// Return as-is if no match
@@ -43,11 +43,7 @@ func (*LinuxContainerEnvironmentExtensions) ToContainerPath(path string) string
 	// Convert to WSL2-compatible path if it is a windows path
 	// NOTE: Cannot use filepath because it will use the wrong path separators assuming we want the path to be windows
 	// based if running on Windows, and because we are feeding this to Docker, GoLang auto-path-translate doesn't work.
-	driveLetter := strings.ToLower(windowsPathComponents[1])
-	translatedPath := strings.ReplaceAll(windowsPathComponents[2], `\`, `/`)
-	// Should make something like /mnt/c/Users/person/My Folder/MyActProject
-	result := strings.Join([]string{"/mnt", driveLetter, translatedPath}, `/`)
-	return result
+	return "/mnt/" + strings.ToLower(windowsPathComponents[1]) + "/" + strings.ReplaceAll(windowsPathComponents[2], `\`, `/`)
 }
 
 func (*LinuxContainerEnvironmentExtensions) GetActPath() string {
@@ -69,10 +65,41 @@ func (*LinuxContainerEnvironmentExtensions) JoinPathVariable(paths ...string) st
 // DefaultToolCache is where the runner mounts the tool cache inside job containers.
 const DefaultToolCache = "/opt/hostedtoolcache"
 
+type runnerArchKey struct{}
+
+func WithRunnerArch(ctx context.Context) context.Context { // shares one architecture lookup per run
+	if ctx.Value(runnerArchKey{}) != nil {
+		return ctx
+	}
+	return context.WithValue(ctx, runnerArchKey{}, &runnerArchCache{lock: make(chan struct{}, 1)})
+}
+
+type runnerArchCache struct {
+	lock chan struct{} // lets cancelled jobs stop waiting for the cache
+	arch string
+}
+
+func cachedRunnerArch(ctx context.Context) string {
+	cache, ok := ctx.Value(runnerArchKey{}).(*runnerArchCache)
+	if !ok {
+		return RunnerArch(ctx)
+	}
+	select {
+	case cache.lock <- struct{}{}:
+		defer func() { <-cache.lock }()
+	case <-ctx.Done():
+		return ""
+	}
+	if cache.arch == "" {
+		cache.arch = RunnerArch(ctx)
+	}
+	return cache.arch
+}
+
 func (*LinuxContainerEnvironmentExtensions) GetRunnerContext(ctx context.Context) map[string]any {
 	return map[string]any{
 		"os":         "Linux",
-		"arch":       RunnerArch(ctx),
+		"arch":       cachedRunnerArch(ctx),
 		"temp":       "/tmp",
 		"tool_cache": DefaultToolCache,
 	}

act/container/linux_container_environment_extensions_test.go

@@ -5,15 +5,13 @@
 package container
 
 import (
-	"fmt"
 	"os"
 	"runtime"
 	"strings"
 	"testing"
 
-	"gitea.com/gitea/runner/act/common/logging"
-
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestContainerPath(t *testing.T) {
@@ -23,42 +21,27 @@ func TestContainerPath(t *testing.T) {
 		workDir         string
 	}
 
-	linuxcontainerext := &LinuxContainerEnvironmentExtensions{}
-
-	if runtime.GOOS == "windows" {
 	cwd, err := os.Getwd()
-		if err != nil {
-			logging.Error(err)
-		}
-
-		rootDrive := os.Getenv("SystemDrive")
-		rootDriveLetter := strings.ReplaceAll(strings.ToLower(rootDrive), `:`, "")
-		for _, v := range []containerPathJob{
-			{"/mnt/c/Users/act/go/src/github.com/nektos/act", "C:\\Users\\act\\go\\src\\github.com\\nektos\\act\\", ""},
-			{"/mnt/f/work/dir", `F:\work\dir`, ""},
-			{"/mnt/c/windows/to/unix", "windows\\to\\unix", rootDrive + "\\"},
-			{fmt.Sprintf("/mnt/%v/act", rootDriveLetter), "act", rootDrive + "\\"},
-		} {
-			if v.workDir != "" {
-				t.Chdir(v.workDir)
-			}
-
-			assert.Equal(t, v.destinationPath, linuxcontainerext.ToContainerPath(v.sourcePath))
-		}
-
-		t.Chdir(cwd)
-	} else {
-		cwd, err := os.Getwd()
-		if err != nil {
-			logging.Error(err)
-		}
-		for _, v := range []containerPathJob{
+	require.NoError(t, err)
+	tests := []containerPathJob{
 		{"/home/act/go/src/github.com/nektos/act", "/home/act/go/src/github.com/nektos/act", ""},
 		{"/home/act", `/home/act/`, ""},
 		{cwd, ".", ""},
-		} {
-			assert.Equal(t, v.destinationPath, linuxcontainerext.ToContainerPath(v.sourcePath))
 	}
+	if runtime.GOOS == "windows" {
+		rootDrive := os.Getenv("SystemDrive")
+		tests = []containerPathJob{
+			{"/mnt/c/Users/act/go/src/github.com/nektos/act", "C:\\Users\\act\\go\\src\\github.com\\nektos\\act\\", ""},
+			{"/mnt/f/work/dir", `F:\work\dir`, ""},
+			{"/mnt/c/windows/to/unix", "windows\\to\\unix", rootDrive + "\\"},
+			{"/mnt/" + strings.ReplaceAll(strings.ToLower(rootDrive), `:`, "") + "/act", "act", rootDrive + "\\"},
+		}
+	}
+	for _, test := range tests {
+		if test.workDir != "" {
+			t.Chdir(test.workDir)
+		}
+		assert.Equal(t, test.destinationPath, (&LinuxContainerEnvironmentExtensions{}).ToContainerPath(test.sourcePath))
 	}
 }
 
@@ -67,5 +50,4 @@ type typeAssertMockContainer struct {
 	LinuxContainerEnvironmentExtensions
 }
 
-// Type assert Container + LinuxContainerEnvironmentExtensions implements ExecutionsEnvironment
 var _ ExecutionsEnvironment = &typeAssertMockContainer{}

act/container/parse_env_file.go

@@ -75,7 +75,6 @@ func parseEnvFile(e Container, srcPath string, env *map[string]string) common.Ex
 		if err := s.Err(); err != nil {
 			return fmt.Errorf("reading env file: %w", err)
 		}
-		env = &localEnv
 		return nil
 	}
 }

act/container/parse_env_file_test.go

@@ -12,7 +12,6 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"golang.org/x/text/encoding"
 	"golang.org/x/text/encoding/unicode"
 )
 
@@ -22,42 +21,40 @@ func newTestHostEnv(t *testing.T) (*HostEnvironment, string) {
 	return e, filepath.Join(e.Path, "envfile")
 }
 
-func TestParseEnvFileSingleLine(t *testing.T) {
-	e, envPath := newTestHostEnv(t)
-	require.NoError(t, os.WriteFile(envPath, []byte("FOO=bar\nBAZ=qux\n"), 0o600))
-
-	env := map[string]string{}
-	require.NoError(t, parseEnvFile(e, envPath, &env)(t.Context()))
-	assert.Equal(t, "bar", env["FOO"])
-	assert.Equal(t, "qux", env["BAZ"])
+func TestParseEnvFile(t *testing.T) {
+	largeValue := strings.Repeat("x", 2*1024*1024)
+	encodeUTF16 := func(endianness unicode.Endianness) string {
+		content, err := unicode.UTF16(endianness, unicode.UseBOM).NewEncoder().Bytes([]byte("FOO=bar\r\nMULTI<<EOF\r\nline1\r\nEOF\r\n"))
+		require.NoError(t, err)
+		return string(content)
 	}
-
-func TestParseEnvFileMultiLine(t *testing.T) {
+	for _, test := range []struct {
+		name, content string
+		want          map[string]string
+	}{
+		{"single line", "FOO=bar\nBAZ=qux\n", map[string]string{"FOO": "bar", "BAZ": "qux"}},
+		{"blank lines", "\nFOO=bar\n\n   \nBAZ=qux\n\n", map[string]string{"FOO": "bar", "BAZ": "qux"}},
+		{"multiline", "FOO<<EOF\nline1\nline2\nEOF\n", map[string]string{"FOO": "line1\nline2"}},
+		{"multiline blank lines", "FOO<<EOF\nline1\n\nline2\nEOF\n", map[string]string{"FOO": "line1\n\nline2"}},
+		{"multiline leading blank line", "FOO<<EOF\n\nline2\nEOF\n", map[string]string{"FOO": "\nline2"}},
+		{"large value within limit", "FOO<<EOF\n" + largeValue + "\nEOF\n", map[string]string{"FOO": largeValue}},
+		{"UTF8 BOM", "\xef\xbb\xbfFOO=bar\n", map[string]string{"FOO": "bar"}},
+		{"UTF16 little endian", encodeUTF16(unicode.LittleEndian), map[string]string{"FOO": "bar", "MULTI": "line1"}},
+		{"UTF16 big endian", encodeUTF16(unicode.BigEndian), map[string]string{"FOO": "bar", "MULTI": "line1"}},
+	} {
+		t.Run(test.name, func(t *testing.T) {
 			e, envPath := newTestHostEnv(t)
-	content := "FOO<<EOF\nline1\nline2\nEOF\n"
-	require.NoError(t, os.WriteFile(envPath, []byte(content), 0o600))
-
+			require.NoError(t, os.WriteFile(envPath, []byte(test.content), 0o600))
 			env := map[string]string{}
 			require.NoError(t, parseEnvFile(e, envPath, &env)(t.Context()))
-	assert.Equal(t, "line1\nline2", env["FOO"])
+			assert.Equal(t, test.want, env)
+		})
 	}
-
-func TestParseEnvFileLargeValueWithinLimit(t *testing.T) {
-	e, envPath := newTestHostEnv(t)
-	big := strings.Repeat("x", 2*1024*1024)
-	content := "FOO<<EOF\n" + big + "\nEOF\n"
-	require.NoError(t, os.WriteFile(envPath, []byte(content), 0o600))
-
-	env := map[string]string{}
-	require.NoError(t, parseEnvFile(e, envPath, &env)(t.Context()))
-	assert.Equal(t, big, env["FOO"])
 }
 
 func TestParseEnvFileLineExceedsBufferReportsScannerError(t *testing.T) {
 	e, envPath := newTestHostEnv(t)
-	tooBig := strings.Repeat("x", 17*1024*1024) // over the 16 MiB cap
-	content := "FOO<<EOF\n" + tooBig + "\nEOF\n"
-	require.NoError(t, os.WriteFile(envPath, []byte(content), 0o600))
+	require.NoError(t, os.WriteFile(envPath, []byte("FOO<<EOF\n"+strings.Repeat("x", 17*1024*1024)+"\nEOF\n"), 0o600))
 
 	env := map[string]string{}
 	err := parseEnvFile(e, envPath, &env)(t.Context())
@@ -65,68 +62,6 @@ func TestParseEnvFileLineExceedsBufferReportsScannerError(t *testing.T) {
 	assert.Contains(t, err.Error(), "reading env file")
 }
 
-// Regression test: a blank line used to fail the job at "Complete Job", after
-// every step had already been recorded as successful.
-func TestParseEnvFileBlankLines(t *testing.T) {
-	e, envPath := newTestHostEnv(t)
-	require.NoError(t, os.WriteFile(envPath, []byte("\nFOO=bar\n\n   \nBAZ=qux\n\n"), 0o600))
-
-	env := map[string]string{}
-	require.NoError(t, parseEnvFile(e, envPath, &env)(t.Context()))
-	assert.Equal(t, "bar", env["FOO"])
-	assert.Equal(t, "qux", env["BAZ"])
-}
-
-// blank lines inside a heredoc value are content, not separators
-func TestParseEnvFileMultiLineKeepsBlankLines(t *testing.T) {
-	e, envPath := newTestHostEnv(t)
-	require.NoError(t, os.WriteFile(envPath, []byte("FOO<<EOF\nline1\n\nline2\nEOF\n"), 0o600))
-
-	env := map[string]string{}
-	require.NoError(t, parseEnvFile(e, envPath, &env)(t.Context()))
-	assert.Equal(t, "line1\n\nline2", env["FOO"])
-
-	require.NoError(t, os.WriteFile(envPath, []byte("FOO<<EOF\n\nline2\nEOF\n"), 0o600))
-	env = map[string]string{}
-	require.NoError(t, parseEnvFile(e, envPath, &env)(t.Context()))
-	assert.Equal(t, "\nline2", env["FOO"])
-}
-
-func TestParseEnvFileUTF8BOM(t *testing.T) {
-	e, envPath := newTestHostEnv(t)
-	content := append([]byte{0xEF, 0xBB, 0xBF}, []byte("FOO=bar\n")...)
-	require.NoError(t, os.WriteFile(envPath, content, 0o600))
-
-	env := map[string]string{}
-	require.NoError(t, parseEnvFile(e, envPath, &env)(t.Context()))
-	assert.Equal(t, "bar", env["FOO"])
-}
-
-// Windows host mode: PowerShell 5.1 redirection writes UTF-16, which used to be
-// unrecognisable as KEY=VALUE, so the writes were silently ignored.
-func TestParseEnvFileUTF16(t *testing.T) {
-	tests := []struct {
-		name    string
-		encoder *encoding.Encoder
-	}{
-		{"little endian", unicode.UTF16(unicode.LittleEndian, unicode.UseBOM).NewEncoder()},
-		{"big endian", unicode.UTF16(unicode.BigEndian, unicode.UseBOM).NewEncoder()},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			e, envPath := newTestHostEnv(t)
-			content, err := tt.encoder.Bytes([]byte("FOO=bar\r\nMULTI<<EOF\r\nline1\r\nEOF\r\n"))
-			require.NoError(t, err)
-			require.NoError(t, os.WriteFile(envPath, content, 0o600))
-
-			env := map[string]string{}
-			require.NoError(t, parseEnvFile(e, envPath, &env)(t.Context()))
-			assert.Equal(t, "bar", env["FOO"])
-			assert.Equal(t, "line1", env["MULTI"])
-		})
-	}
-}
-
 func TestParseEnvFileMissingDelimiter(t *testing.T) {
 	e, envPath := newTestHostEnv(t)
 	require.NoError(t, os.WriteFile(envPath, []byte("FOO<<EOF\nline1\nline2\n"), 0o600))