fix(e2e): make act container start and CLI tar-copy deterministic

e2e/harness/act.go

@@ -37,6 +37,19 @@ type RunOpts struct {
 	RepoPath        string            // Path to cloned repo in container
 }
 
+// actStartupTimeout bounds how long the container readiness probe waits for the
+// act container to become runnable. testcontainers' default is 60s, which is
+// ample for the probe itself but offers no slack when a cold first-run image
+// pull has already consumed wall-clock and daemon bandwidth. A generous-but-
+// finite budget tolerates the one-time cold pull while still failing closed if
+// the container never becomes ready.
+const actStartupTimeout = 5 * time.Minute
+
+// actStartupPollInterval is how often the readiness probe re-checks while
+// waiting. A brisk interval detects a healthy container promptly once the pull
+// completes, without hammering the daemon.
+const actStartupPollInterval = 2 * time.Second
+
 // NewActRunner starts a new act container
 func NewActRunner(ctx context.Context, giteaURL, giteaToken, networkName string, net *testcontainers.DockerNetwork) (*ActRunner, error) {
 	var networks []string
@@ -50,10 +63,22 @@ func NewActRunner(ctx context.Context, giteaURL, giteaToken, networkName string,
 	// network alias directly. Job containers are configured separately
 	// in actrc below.
 	req := testcontainers.ContainerRequest{
-		Image:      "ghcr.io/catthehacker/ubuntu:act-latest",
-		Cmd:        []string{"sleep", "infinity"}, // Keep container running
-		Networks:   networks,
-		WaitingFor: wait.ForExec([]string{"echo", "ready"}),
+		Image:    "ghcr.io/catthehacker/ubuntu:act-latest",
+		Cmd:      []string{"sleep", "infinity"}, // Keep container running
+		Networks: networks,
+		// The readiness exec ("echo ready") only proves the container is up; it
+		// is near-instant once the container is running. The slow, variable part
+		// is the FIRST-RUN image pull, which testcontainers performs as part of
+		// container creation, before this wait strategy's clock starts. The
+		// strategy's own startup budget defaults to 60s, which is fine for the
+		// exec itself but leaves no slack when a cold pull has already eaten into
+		// the daemon's bandwidth. Give the readiness probe an explicit,
+		// generous budget (still a hard cap: a container that never becomes
+		// runnable fails after actStartupTimeout) and poll briskly so a healthy
+		// container is detected promptly.
+		WaitingFor: wait.ForExec([]string{"echo", "ready"}).
+			WithStartupTimeout(actStartupTimeout).
+			WithPollInterval(actStartupPollInterval),
 		HostConfigModifier: func(hc *container.HostConfig) {
 			hc.Mounts = append(hc.Mounts, mount.Mount{
 				Type:   mount.TypeBind,

e2e/harness/cli_copy_test.go

@@ -0,0 +1,136 @@
+package harness
+
+import (
+	"archive/tar"
+	"bytes"
+	"context"
+	"errors"
+	"io"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// fakeCopier records CopyToContainer calls and fails the first failUntil of
+// them, letting tests drive copyCLIToContainer's retry path without Docker.
+type fakeCopier struct {
+	failUntil int   // number of leading attempts that return err
+	err       error // error returned while failing
+	calls     int   // total CopyToContainer invocations
+	lastBytes []byte
+	lastPath  string
+}
+
+func (f *fakeCopier) CopyToContainer(_ context.Context, content []byte, path string, _ int64) error {
+	f.calls++
+	f.lastBytes = content
+	f.lastPath = path
+	if f.calls <= f.failUntil {
+		return f.err
+	}
+	return nil
+}
+
+func TestCopyCLIToContainer_SucceedsFirstTry(t *testing.T) {
+	t.Parallel()
+
+	fc := &fakeCopier{}
+	content := []byte("cascade-binary-bytes")
+
+	err := copyCLIToContainer(context.Background(), fc, content, "/usr/local/bin/cascade")
+
+	require.NoError(t, err)
+	assert.Equal(t, 1, fc.calls, "a clean copy must not retry")
+	assert.Equal(t, content, fc.lastBytes)
+	assert.Equal(t, "/usr/local/bin/cascade", fc.lastPath)
+}
+
+func TestCopyCLIToContainer_RetriesTransientThenSucceeds(t *testing.T) {
+	// Zero the backoff so the retry path runs fast.
+	orig := cliCopyBackoff
+	cliCopyBackoff = func(int) time.Duration { return 0 }
+	t.Cleanup(func() { cliCopyBackoff = orig })
+
+	fc := &fakeCopier{failUntil: cliCopyMaxAttempts - 1, err: errors.New("docker daemon connection reset")}
+
+	err := copyCLIToContainer(context.Background(), fc, []byte("x"), "/dst")
+
+	require.NoError(t, err, "a copy that succeeds within the budget must not surface an error")
+	assert.Equal(t, cliCopyMaxAttempts, fc.calls)
+}
+
+func TestCopyCLIToContainer_FailsClosedAfterBudget(t *testing.T) {
+	orig := cliCopyBackoff
+	cliCopyBackoff = func(int) time.Duration { return 0 }
+	t.Cleanup(func() { cliCopyBackoff = orig })
+
+	sentinel := errors.New("archive/tar: write too long")
+	fc := &fakeCopier{failUntil: cliCopyMaxAttempts + 5, err: sentinel}
+
+	err := copyCLIToContainer(context.Background(), fc, []byte("x"), "/dst")
+
+	require.Error(t, err, "an exhausted retry budget must surface an error, not swallow it")
+	assert.ErrorIs(t, err, sentinel, "the underlying copy error must be wrapped, not lost")
+	assert.Equal(t, cliCopyMaxAttempts, fc.calls, "retries are bounded by cliCopyMaxAttempts")
+}
+
+func TestCopyCLIToContainer_HonorsContextCancellation(t *testing.T) {
+	// Keep a real backoff so the cancelled context is observed during the wait.
+	fc := &fakeCopier{failUntil: cliCopyMaxAttempts, err: errors.New("blip")}
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel() // cancelled before the first retry's backoff
+
+	err := copyCLIToContainer(ctx, fc, []byte("x"), "/dst")
+
+	require.Error(t, err)
+	assert.ErrorIs(t, err, context.Canceled)
+	assert.Equal(t, 1, fc.calls, "cancellation must stop further attempts")
+}
+
+// TestStableSliceTarSizeMatches proves the invariant the #336 fix relies on:
+// when the tar header size and the payload both come from one immutable byte
+// slice, archive/tar writes exactly Size bytes with no "write too long" or
+// short-write mismatch. This is the deterministic property CopyToContainer
+// gives us that CopyFileToContainer (stat-then-stream) does not.
+func TestStableSliceTarSizeMatches(t *testing.T) {
+	t.Parallel()
+
+	content := bytes.Repeat([]byte("cascade"), 4096) // arbitrary stable payload
+
+	var buf bytes.Buffer
+	tw := tar.NewWriter(&buf)
+	require.NoError(t, tw.WriteHeader(&tar.Header{
+		Name: "usr/local/bin/cascade",
+		Mode: 0o755,
+		Size: int64(len(content)), // declared size derives from the SAME slice
+	}))
+	n, err := tw.Write(content)
+	require.NoError(t, err, "writing exactly Size bytes must not error")
+	require.NoError(t, tw.Close(), "closing after a size-matched write must not error")
+	assert.Equal(t, len(content), n)
+
+	// Round-trip: read the archive back and confirm the bytes are intact.
+	tr := tar.NewReader(&buf)
+	hdr, err := tr.Next()
+	require.NoError(t, err)
+	assert.Equal(t, int64(len(content)), hdr.Size)
+	got, err := io.ReadAll(tr)
+	require.NoError(t, err)
+	assert.Equal(t, content, got)
+}
+
+func TestActStartupBudgets(t *testing.T) {
+	t.Parallel()
+
+	// The readiness probe must carry a cold-pull-aware budget that is still a
+	// hard cap, and poll briskly. These guard against a silent regression to the
+	// 60s testcontainers default that let the cold-image start flake (#338).
+	assert.GreaterOrEqual(t, actStartupTimeout, 3*time.Minute,
+		"startup budget must tolerate a one-time cold image pull")
+	assert.Greater(t, actStartupPollInterval, time.Duration(0),
+		"poll interval must be positive")
+	assert.Less(t, actStartupPollInterval, actStartupTimeout,
+		"poll interval must be far smaller than the overall budget")
+}

e2e/harness/harness.go

@@ -27,10 +27,20 @@ import (
 // and downstream orchestrate steps to fail with "No such file or directory".
 // Build once per process under sync.Once and reuse a stable, package-private
 // path; all scenarios share the same source so the binary is identical.
+//
+// We also cache the built binary's BYTES, not just its path. testcontainers'
+// CopyFileToContainer re-Stats the file and then streams it, declaring the
+// stat'd size in the tar header before writing the bytes. If anything mutates
+// the file in the gap between the stat and the stream, the bytes written no
+// longer match the declared header size and archive/tar fails the copy with
+// "write too long" (issue #336). Copying from a stable in-memory slice via
+// CopyToContainer makes the declared size and the written bytes derive from the
+// same immutable buffer, so that size-mismatch window cannot exist.
 var (
-	cliBinaryOnce sync.Once
-	cliBinaryPath string
-	cliBinaryErr  error
+	cliBinaryOnce  sync.Once
+	cliBinaryPath  string
+	cliBinaryBytes []byte
+	cliBinaryErr   error
 )
 
 // normalizeCallbackStubPath returns the canonical path where a callback stub
@@ -508,14 +518,14 @@ func (h *Harness) GenerateWorkflows(ctx context.Context) error {
 	}
 
 	// Build CLI once for the whole test process and copy into this scenario's
-	// act container.
-	binaryPath, err := h.ensureCLIBinary(ctx)
-	if err != nil {
+	// act container. The copy streams the cached binary bytes (not the on-disk
+	// file) to avoid the archive/tar size-mismatch race (#336).
+	if _, err := h.ensureCLIBinary(ctx); err != nil {
 		return err
 	}
 
-	if err := h.act.Container().CopyFileToContainer(ctx, binaryPath, "/usr/local/bin/cascade", 0755); err != nil {
-		return fmt.Errorf("failed to copy CLI to container: %w", err)
+	if err := copyCLIToContainer(ctx, h.act.Container(), cliBinaryBytes, "/usr/local/bin/cascade"); err != nil {
+		return err
 	}
 
 	// Also copy the CLI into the repo so job containers can access it
@@ -833,11 +843,72 @@ func (h *Harness) ensureCLIBinary(ctx context.Context) (string, error) {
 			cliBinaryErr = fmt.Errorf("failed to build CLI: %w\nOutput: %s", err, output)
 			return
 		}
+
+		// Read the freshly built binary into a stable in-memory slice once, so
+		// every container copy streams from an immutable buffer rather than
+		// re-reading (and re-stat'ing) the on-disk file. This is what closes the
+		// archive/tar size-mismatch race (#336): the tar header size and the
+		// payload bytes both come from this single slice.
+		data, err := os.ReadFile(path)
+		if err != nil {
+			_ = os.Remove(path)
+			cliBinaryErr = fmt.Errorf("failed to read built CLI: %w", err)
+			return
+		}
 		cliBinaryPath = path
+		cliBinaryBytes = data
 	})
 	return cliBinaryPath, cliBinaryErr
 }
 
+// cliCopyMaxAttempts bounds how many times copyCLIToContainer retries the tar
+// copy. The copy itself is deterministic now that it streams from a stable
+// in-memory slice, so a retry only absorbs a genuine, transient Docker daemon
+// API blip (a dropped connection mid-copy, a momentary daemon stall). A small
+// fixed budget clears that; a copy that keeps failing surfaces the real error.
+const cliCopyMaxAttempts = 3
+
+// cliCopyBackoff returns the pause before the retry that follows a failed copy
+// attempt (attempt is 1-based). It is a var so tests can zero the wait. The
+// growth (250ms, 500ms) lets a momentary daemon stall clear without stalling
+// the suite.
+var cliCopyBackoff = func(attempt int) time.Duration {
+	return time.Duration(attempt) * 250 * time.Millisecond
+}
+
+// containerCopier is the slice of the container surface copyCLIToContainer
+// needs: copying a byte slice to an in-container path. The real
+// testcontainers.Container satisfies it, and a fake satisfies it in tests, so
+// the copy/retry logic is exercisable without Docker.
+type containerCopier interface {
+	CopyToContainer(ctx context.Context, fileContent []byte, containerFilePath string, fileMode int64) error
+}
+
+// copyCLIToContainer copies the cached CLI binary bytes into the container at
+// destPath, retrying a transient Docker daemon copy blip up to
+// cliCopyMaxAttempts. It streams from the stable cliBinaryBytes slice via
+// CopyToContainer, so the archive/tar header size always matches the payload
+// (the #336 root cause). The bounded retry only covers a genuine transport
+// blip and fails closed, surfacing the last error once the budget is spent.
+func copyCLIToContainer(ctx context.Context, dst containerCopier, content []byte, destPath string) error {
+	var lastErr error
+	for attempt := 1; attempt <= cliCopyMaxAttempts; attempt++ {
+		if attempt > 1 {
+			select {
+			case <-ctx.Done():
+				return fmt.Errorf("CLI copy cancelled after %d attempt(s): %w", attempt-1, ctx.Err())
+			case <-time.After(cliCopyBackoff(attempt - 1)):
+			}
+		}
+		err := dst.CopyToContainer(ctx, content, destPath, 0o755)
+		if err == nil {
+			return nil
+		}
+		lastErr = err
+	}
+	return fmt.Errorf("failed to copy CLI to container after %d attempt(s): %w", cliCopyMaxAttempts, lastErr)
+}
+
 // localizeWorkflows rewrites generated workflows so external action refs
 // (`stablekernel/cascade/.github/actions/X@ref`) point at the local
 // mock actions. A second pass adds the `./` prefix act needs for any bare

e2e/harness/multi_repo.go

@@ -272,13 +272,14 @@ func (h *MultiRepoHarness) prepareRepoInActContainer(ctx context.Context, repoCt
 	}
 
 	// Copy the (already-built) CLI binary into the repo so the mock setup-cli
-	// action can install it onto PATH inside job containers.
-	binaryPath, err := h.base.ensureCLIBinary(ctx)
-	if err != nil {
+	// action can install it onto PATH inside job containers. The copy streams
+	// the cached binary bytes (not the on-disk file) to avoid the archive/tar
+	// size-mismatch race (#336).
+	if _, err := h.base.ensureCLIBinary(ctx); err != nil {
 		return err
 	}
-	if err := h.act.Container().CopyFileToContainer(ctx, binaryPath, "/usr/local/bin/cascade", 0755); err != nil {
-		return fmt.Errorf("failed to copy CLI to container: %w", err)
+	if err := copyCLIToContainer(ctx, h.act.Container(), cliBinaryBytes, "/usr/local/bin/cascade"); err != nil {
+		return err
 	}
 	copyToRepoCmd := []string{
 		"bash", "-c",