feat(hooks): retain failed pre_start containers for post-mortem

A failing pre_start hook container is now retained (AutoRemove: false)
so operators can run 'docker logs <id>' and 'docker ps -a' to diagnose
the failure. On success the container is removed explicitly, mirroring
the old AutoRemove behaviour including anonymous volumes.

Before each pre_start run, stale hook containers from a previous failed
run are detected via project+service+HookLabel filters and force-removed
so they do not accumulate.

Changes:
- pkg/api/labels.go: add HookLabel (com.docker.compose.hook)
- pkg/compose/filters.go: add hookFilter helper
- pkg/compose/pre_start.go:
  - AutoRemove: false in createPreStartContainer
  - HookLabel added to container labels
  - runPreStartHook: explicit ContainerRemove on success; retain on failure
  - removeOrphanPreStartContainers: new helper called in runPreStart
- Tests: update all existing pre_start tests for the new flow; add
  feature tests (success removes, failure retains, orphan cleanup) and
  coverage-gap tests (lowestNumberedContainer, waitPreStart cancel,
  preStartResultErr, streamPreStartLogs error paths, old-API network
  paths, ExecCreate/Attach/Inspect errors, hookExitError branches)

Coverage after: hook.go 100%, pre_start.go most functions 100%
(was 89% and 64% respectively per Codecov delta).

Signed-off-by: Guillaume Lours <[email protected]>

pkg/api/labels.go

@@ -62,6 +62,11 @@ const (
 	ContainerReplaceLabel = "com.docker.compose.replace"
 	// ContainerEngineLabel stores the name of the engine that runs the container
 	ContainerEngineLabel = "com.docker.compose.engine"
+	// HookLabel identifies an ephemeral lifecycle-hook container and stores the
+	// hook type, e.g. "pre_start". It is applied to every container created by
+	// runPreStartHook so orphan hook containers from a previous failed run can
+	// be found and removed by project+service+hook label filters.
+	HookLabel = "com.docker.compose.hook"
 )
 
 // ComposeVersion is the compose tool version as declared by label VersionLabel

pkg/compose/filters.go

@@ -53,3 +53,7 @@ func oneOffFilter(b bool) string {
 func containerNumberFilter(index int) string {
 	return labelFilter(api.ContainerNumberLabel, strconv.Itoa(index))
 }
+
+func hookFilter(hookType string) string {
+	return labelFilter(api.HookLabel, hookType)
+}

pkg/compose/hook_test.go

@@ -19,8 +19,10 @@
 package compose
 
 import (
+	"bufio"
 	"context"
 	"encoding/binary"
+	"fmt"
 	"io"
 	"net"
 	"os"
@@ -434,3 +436,164 @@ func TestRunHook_ContextCancellation(t *testing.T) {
 		t.Fatal("runHook did not return within 5 s after ctx cancellation")
 	}
 }
+
+// TestRunHook_CopyError covers the branch in runHook where the copy goroutine
+// itself returns an error (e.g. a mid-stream I/O failure on the exec reader).
+// The error must be propagated immediately without blocking.
+func TestRunHook_CopyError(t *testing.T) {
+	mockCtrl := gomock.NewController(t)
+	defer mockCtrl.Finish()
+	mockAPI := mocks.NewMockAPIClient(mockCtrl)
+	mockCli := mocks.NewMockCli(mockCtrl)
+	mockCli.EXPECT().Client().Return(mockAPI).AnyTimes()
+	mockCli.EXPECT().Err().Return(streams.NewOut(os.Stderr)).AnyTimes()
+	mockCli.EXPECT().Out().Return(streams.NewOut(os.Stdout)).AnyTimes()
+
+	ctr := container.Summary{ID: "ctr-1"}
+	service := types.ServiceConfig{Name: "svc", Tty: false}
+	hook := types.ServiceHook{Command: types.ShellCommand{"true"}}
+
+	mockAPI.EXPECT().
+		ExecCreate(gomock.Any(), "ctr-1", gomock.Any()).
+		Return(client.ExecCreateResult{ID: "exec-1"}, nil)
+
+	// Build a pipe whose read side immediately returns a hard error, simulating
+	// an I/O failure mid-stream rather than a clean EOF.
+	pr, pw := io.Pipe()
+	_ = pw.CloseWithError(fmt.Errorf("simulated I/O failure"))
+
+	// Use net.Pipe() only for the Conn field (Close); reads come from pr.
+	serverConn, clientConn := net.Pipe()
+	defer serverConn.Close() //nolint:errcheck
+	defer clientConn.Close() //nolint:errcheck
+
+	mockAPI.EXPECT().
+		ExecAttach(gomock.Any(), "exec-1", gomock.Any()).
+		Return(client.ExecAttachResult{
+			HijackedResponse: client.HijackedResponse{
+				Conn:   clientConn,
+				Reader: bufio.NewReader(pr),
+			},
+		}, nil)
+	// ExecInspect must NOT be called: runHook must return the copy error first.
+
+	s, err := NewComposeService(mockCli)
+	assert.NilError(t, err)
+
+	err = s.(*composeService).runHook(t.Context(), ctr, service, hook, nil)
+	assert.ErrorContains(t, err, "simulated I/O failure")
+}
+
+// TestHookExitError_NoOutput covers the branch in hookExitError where both
+// stdout and stderr tails are empty — the error must not include an output suffix.
+func TestHookExitError_NoOutput(t *testing.T) {
+	empty1 := newOutputTail(10, 512)
+	empty2 := newOutputTail(10, 512)
+	err := hookExitError("mysvc", 127, empty1, empty2)
+	assert.ErrorContains(t, err, "mysvc")
+	assert.ErrorContains(t, err, "127")
+	// No colon separator: the no-output format has no ': <output>' suffix.
+	assert.Assert(t, !strings.Contains(err.Error(), ": "), "unexpected output suffix in: %q", err.Error())
+}
+
+// TestHookExitError_StdoutFallback covers the branch in hookExitError where
+// stderr is empty but stdout has content: stdout is used as the fallback output.
+func TestHookExitError_StdoutFallback(t *testing.T) {
+	stdoutTail := newOutputTail(10, 512)
+	_, _ = stdoutTail.Write([]byte("stdout only line\n"))
+	stderrTail := newOutputTail(10, 512) // empty
+
+	err := hookExitError("mysvc", 1, stdoutTail, stderrTail)
+	assert.ErrorContains(t, err, "stdout only line")
+}
+
+// TestRunHook_ExecCreateError covers the branch where ExecCreate returns an
+// error — runHook must propagate it immediately.
+func TestRunHook_ExecCreateError(t *testing.T) {
+	mockCtrl := gomock.NewController(t)
+	defer mockCtrl.Finish()
+	mockAPI := mocks.NewMockAPIClient(mockCtrl)
+	mockCli := mocks.NewMockCli(mockCtrl)
+	mockCli.EXPECT().Client().Return(mockAPI).AnyTimes()
+	mockCli.EXPECT().Err().Return(streams.NewOut(os.Stderr)).AnyTimes()
+	mockCli.EXPECT().Out().Return(streams.NewOut(os.Stdout)).AnyTimes()
+
+	ctr := container.Summary{ID: "ctr-1"}
+	service := types.ServiceConfig{Name: "svc"}
+	hook := types.ServiceHook{Command: types.ShellCommand{"true"}}
+
+	mockAPI.EXPECT().
+		ExecCreate(gomock.Any(), "ctr-1", gomock.Any()).
+		Return(client.ExecCreateResult{}, fmt.Errorf("exec create failed"))
+
+	s, err := NewComposeService(mockCli)
+	assert.NilError(t, err)
+	err = s.(*composeService).runHook(t.Context(), ctr, service, hook, nil)
+	assert.ErrorContains(t, err, "exec create failed")
+}
+
+// TestRunHook_ExecAttachError covers the branch where ExecAttach returns an
+// error — runHook must propagate it immediately.
+func TestRunHook_ExecAttachError(t *testing.T) {
+	mockCtrl := gomock.NewController(t)
+	defer mockCtrl.Finish()
+	mockAPI := mocks.NewMockAPIClient(mockCtrl)
+	mockCli := mocks.NewMockCli(mockCtrl)
+	mockCli.EXPECT().Client().Return(mockAPI).AnyTimes()
+	mockCli.EXPECT().Err().Return(streams.NewOut(os.Stderr)).AnyTimes()
+	mockCli.EXPECT().Out().Return(streams.NewOut(os.Stdout)).AnyTimes()
+
+	ctr := container.Summary{ID: "ctr-1"}
+	service := types.ServiceConfig{Name: "svc"}
+	hook := types.ServiceHook{Command: types.ShellCommand{"true"}}
+
+	mockAPI.EXPECT().
+		ExecCreate(gomock.Any(), "ctr-1", gomock.Any()).
+		Return(client.ExecCreateResult{ID: "exec-1"}, nil)
+	mockAPI.EXPECT().
+		ExecAttach(gomock.Any(), "exec-1", gomock.Any()).
+		Return(client.ExecAttachResult{}, fmt.Errorf("exec attach failed"))
+
+	s, err := NewComposeService(mockCli)
+	assert.NilError(t, err)
+	err = s.(*composeService).runHook(t.Context(), ctr, service, hook, nil)
+	assert.ErrorContains(t, err, "exec attach failed")
+}
+
+// TestRunHook_ExecInspectError covers the branch where ExecInspect returns an
+// error — runHook must propagate it (copy finished successfully beforehand).
+func TestRunHook_ExecInspectError(t *testing.T) {
+	mockCtrl := gomock.NewController(t)
+	defer mockCtrl.Finish()
+	mockAPI := mocks.NewMockAPIClient(mockCtrl)
+	mockCli := mocks.NewMockCli(mockCtrl)
+	mockCli.EXPECT().Client().Return(mockAPI).AnyTimes()
+	mockCli.EXPECT().Err().Return(streams.NewOut(os.Stderr)).AnyTimes()
+	mockCli.EXPECT().Out().Return(streams.NewOut(os.Stdout)).AnyTimes()
+
+	ctr := container.Summary{ID: "ctr-1"}
+	service := types.ServiceConfig{Name: "svc", Tty: false}
+	hook := types.ServiceHook{Command: types.ShellCommand{"true"}}
+
+	mockAPI.EXPECT().
+		ExecCreate(gomock.Any(), "ctr-1", gomock.Any()).
+		Return(client.ExecCreateResult{ID: "exec-1"}, nil)
+
+	// Pipe that closes immediately (EOF) so StdCopy returns with no error.
+	serverConn, clientConn := net.Pipe()
+	serverConn.Close() //nolint:errcheck
+	mockAPI.EXPECT().
+		ExecAttach(gomock.Any(), "exec-1", gomock.Any()).
+		Return(client.ExecAttachResult{
+			HijackedResponse: client.NewHijackedResponse(clientConn, ""),
+		}, nil)
+
+	mockAPI.EXPECT().
+		ExecInspect(gomock.Any(), "exec-1", gomock.Any()).
+		Return(client.ExecInspectResult{}, fmt.Errorf("inspect failed"))
+
+	s, err := NewComposeService(mockCli)
+	assert.NilError(t, err)
+	err = s.(*composeService).runHook(t.Context(), ctr, service, hook, nil)
+	assert.ErrorContains(t, err, "inspect failed")
+}

pkg/compose/pre_start.go

@@ -33,6 +33,11 @@ import (
 	"github.com/docker/compose/v5/pkg/utils"
 )
 
+// preStartHookType is stored in the HookLabel on every pre_start hook container
+// so orphan containers from a previous failed run can be identified and removed
+// by a project+service+hook label filter.
+const preStartHookType = "pre_start"
+
 // lowestNumberedContainer returns the container with the lowest
 // com.docker.compose.container-number label, so pre_start always targets the
 // same replica regardless of the order the daemon returned them in.
@@ -65,6 +70,14 @@ func (s *composeService) runPreStart(ctx context.Context, project *types.Project
 			return fmt.Errorf("service %q pre_start[%d]: per_replica is not yet supported; remove per_replica or set it to false", service.Name, i)
 		}
 	}
+	// Remove any hook containers left behind by a previous failed run so they do
+	// not accumulate. Only one orphan can exist per service (failure gates the
+	// remaining hooks), but we clean the whole set in case the service definition
+	// changed between runs. Removal failures are non-fatal: they are logged so
+	// the operator can identify the stale container.
+	if err := s.removeOrphanPreStartContainers(ctx, project.Name, service.Name); err != nil {
+		logrus.Warnf("service %q: failed to remove stale pre_start hook containers: %v", service.Name, err)
+	}
 	for i, hook := range service.PreStart {
 		if err := s.runPreStartHook(ctx, project, service, ctr, i, hook, listener); err != nil {
 			return err
@@ -89,20 +102,18 @@ func (s *composeService) runPreStartHook(
 		Condition: container.WaitConditionNextExit,
 	})
 
-	// Open the log stream before ContainerStart so AutoRemove cannot race us
-	// to a 404 on a fast-exiting hook. The dedicated logCtx lets us force the
-	// follow stream closed once the hook has exited, so a daemon that keeps
-	// the connection open cannot deadlock `<-logsDone`.
+	// Open the log stream before ContainerStart so a fast-exiting hook cannot
+	// race us to a 404. The dedicated logCtx lets us force the follow stream
+	// closed once the hook has exited, so a daemon that keeps the connection
+	// open cannot deadlock `<-logsDone`.
 	logCtx, cancelLogs := context.WithCancel(ctx)
 	defer cancelLogs()
 	logsDone, getTail := s.streamPreStartLogs(logCtx, created.ID, service, index, listener)
 
 	if _, err := s.apiClient().ContainerStart(ctx, created.ID, client.ContainerStartOptions{}); err != nil {
-		// AutoRemove only fires after a successful start, so the never-started
-		// container has to be dropped explicitly. A failed removal is logged
-		// at warn level — without that hint the orphan is only discoverable
-		// via the project/service labels.
-		if _, removeErr := s.apiClient().ContainerRemove(ctx, created.ID, client.ContainerRemoveOptions{Force: true}); removeErr != nil {
+		// AutoRemove is false, so we must remove the never-started container
+		// explicitly. A failed removal is logged so the orphan is visible.
+		if _, removeErr := s.apiClient().ContainerRemove(ctx, created.ID, client.ContainerRemoveOptions{Force: true, RemoveVolumes: true}); removeErr != nil {
 			logrus.Warnf("service %q pre_start[%d]: failed to remove orphan hook container %s: %v", service.Name, index, created.ID, removeErr)
 		}
 		// Drain waitRes so the client's wait goroutine exits without having to
@@ -121,11 +132,34 @@ func (s *composeService) runPreStartHook(
 	cancelLogs()
 	<-logsDone
 	if waitErr != nil {
-		if tail := getTail(); tail != "" {
-			return fmt.Errorf("%w: %s", waitErr, tail)
+		// Ctrl-C is a user cancellation, not a hook failure: remove the container
+		// and return the raw context error without decorating it with the tail or
+		// retaining the container for post-mortem inspection.
+		if ctx.Err() != nil {
+			if _, removeErr := s.apiClient().ContainerRemove(context.Background(), created.ID, client.ContainerRemoveOptions{Force: true, RemoveVolumes: true}); removeErr != nil {
+				logrus.Warnf("service %q pre_start[%d]: failed to remove hook container %s after cancellation: %v", service.Name, index, created.ID, removeErr)
+			}
+			return waitErr
 		}
+		// Genuine hook failure: retain the container so the operator can run
+		// `docker logs <id>` and `docker inspect <id>` to diagnose the failure.
+		// Include the short container ID in the error to make it actionable.
+		shortID := created.ID
+		if len(shortID) > 12 {
+			shortID = shortID[:12]
+		}
+		if tail := getTail(); tail != "" {
+			return fmt.Errorf("%w: %s (hook container %s retained for inspection)", waitErr, tail, shortID)
+		}
+		return fmt.Errorf("%w (hook container %s retained for inspection)", waitErr, shortID)
 	}
-	return waitErr
+	// Success: remove the hook container, mirroring the old AutoRemove behaviour
+	// (including its anonymous volumes). A removal failure is logged but does not
+	// gate service start — the hook already succeeded.
+	if _, removeErr := s.apiClient().ContainerRemove(ctx, created.ID, client.ContainerRemoveOptions{RemoveVolumes: true}); removeErr != nil {
+		logrus.Warnf("service %q pre_start[%d]: failed to remove hook container %s: %v", service.Name, index, created.ID, removeErr)
+	}
+	return nil
 }
 
 func (s *composeService) createPreStartContainer(
@@ -144,16 +178,21 @@ func (s *composeService) createPreStartContainer(
 		WorkingDir: hook.WorkingDir,
 		Env:        append(ToMobyEnv(service.Environment), ToMobyEnv(hook.Environment)...),
 		// Tag the ephemeral hook container with the project/service it belongs
-		// to so a failed AutoRemove leaves something that `compose down` (and
-		// other label-scoped tooling) can still find.
+		// to so it can be found by `compose down` and label-scoped tooling.
+		// HookLabel also distinguishes hook containers from the real service
+		// container (which shares ProjectLabel and ServiceLabel).
 		Labels: map[string]string{
 			api.ProjectLabel: project.Name,
 			api.ServiceLabel: service.Name,
 			api.VersionLabel: api.ComposeVersion,
+			api.HookLabel:    preStartHookType,
 		},
 	}
 	hostCfg := &container.HostConfig{
-		AutoRemove:  true,
+		// AutoRemove is intentionally false: a failed hook container is retained
+		// so the operator can inspect its logs. On success runPreStartHook
+		// removes the container explicitly (see the success path below).
+		AutoRemove:  false,
 		Privileged:  hook.Privileged,
 		VolumesFrom: []string{ctr.ID},
 	}
@@ -180,10 +219,9 @@ func (s *composeService) createPreStartContainer(
 
 	if versions.LessThan(apiVersion, apiVersion144) {
 		if err := s.connectPreStartExtraNetworks(ctx, project, service, created.ID, networkMode); err != nil {
-			// Same reason as the ContainerStart-failure cleanup: AutoRemove never
-			// fires on a container that was created but not started. Surface
-			// any cleanup failure so the orphan is at least visible in logs.
-			if _, removeErr := s.apiClient().ContainerRemove(ctx, created.ID, client.ContainerRemoveOptions{Force: true}); removeErr != nil {
+			// AutoRemove is false; remove the container explicitly since it was
+			// never started. Log failures so the orphan is at least visible.
+			if _, removeErr := s.apiClient().ContainerRemove(ctx, created.ID, client.ContainerRemoveOptions{Force: true, RemoveVolumes: true}); removeErr != nil {
 				logrus.Warnf("service %q pre_start: failed to remove orphan hook container %s: %v", service.Name, created.ID, removeErr)
 			}
 			return client.ContainerCreateResult{}, err
@@ -215,6 +253,32 @@ func (s *composeService) connectPreStartExtraNetworks(ctx context.Context, proje
 	return nil
 }
 
+// removeOrphanPreStartContainers finds and force-removes any hook containers
+// left behind by a previous failed run of this service's pre_start hooks.
+// Containers are identified by project + service + HookLabel=pre_start.
+// Removal failures are logged at warn level and do not abort the run.
+func (s *composeService) removeOrphanPreStartContainers(ctx context.Context, projectName, serviceName string) error {
+	f := projectFilter(projectName)
+	f.Add("label", serviceFilter(serviceName))
+	f.Add("label", hookFilter(preStartHookType))
+	res, err := s.apiClient().ContainerList(ctx, client.ContainerListOptions{
+		All:     true,
+		Filters: f,
+	})
+	if err != nil {
+		return err
+	}
+	for _, ctr := range res.Items {
+		if _, removeErr := s.apiClient().ContainerRemove(ctx, ctr.ID, client.ContainerRemoveOptions{
+			Force:         true,
+			RemoveVolumes: true,
+		}); removeErr != nil {
+			logrus.Warnf("failed to remove stale pre_start hook container %s: %v", ctr.ID, removeErr)
+		}
+	}
+	return nil
+}
+
 func waitPreStart(ctx context.Context, serviceName string, index int, waitRes client.ContainerWaitResult) error {
 	// ContainerWait can deliver on Result and Error at the same instant. Two
 	// races have to be closed deterministically here:
@@ -262,9 +326,6 @@ func preStartResultErr(serviceName string, index int, res container.WaitResponse
 	return nil
 }
 
-// streamPreStartLogs returns a channel that is closed once the hook log stream
-// has been fully drained (or never opened). Callers must wait on it before
-// returning so the goroutine cannot outlive the hook.
 // streamPreStartLogs opens the hook container's log stream in a background
 // goroutine, tees output into the listener (when non-nil) and into per-stream
 // tail buffers. It returns:

pkg/compose/pre_start_test.go

@@ -18,8 +18,10 @@ package compose
 
 import (
 	"bytes"
+	"context"
 	"fmt"
 	"io"
+	"strings"
 	"testing"
 
 	"github.com/compose-spec/compose-go/v2/types"
@@ -34,11 +36,11 @@ import (
 )
 
 func newPreStartTestService(t *testing.T) (*composeService, *mocks.MockAPIClient) {
+	return newPreStartTestServiceWithVersion(t, "1.44")
+}
+
+func newPreStartTestServiceWithVersion(t *testing.T, apiVersion string) (*composeService, *mocks.MockAPIClient) {
 	t.Helper()
-	// Register the goroutine-leak check first so it runs last (t.Cleanup is
-	// LIFO), after gomock's own cleanup has drained any internal goroutines.
-	// Any goroutine spawned by the code under test that outlives the test will
-	// fail this assertion.
 	ignoreExisting := goleak.IgnoreCurrent()
 	t.Cleanup(func() {
 		goleak.VerifyNone(t, ignoreExisting)
@@ -49,8 +51,8 @@ func newPreStartTestService(t *testing.T) (*composeService, *mocks.MockAPIClient
 	cli := mocks.NewMockCli(mockCtrl)
 	cli.EXPECT().Client().Return(apiClient).AnyTimes()
 	apiClient.EXPECT().Ping(gomock.Any(), client.PingOptions{NegotiateAPIVersion: true}).
-		Return(client.PingResult{APIVersion: "1.44"}, nil).AnyTimes()
-	apiClient.EXPECT().ClientVersion().Return("1.44").AnyTimes()
+		Return(client.PingResult{APIVersion: apiVersion}, nil).AnyTimes()
+	apiClient.EXPECT().ClientVersion().Return(apiVersion).AnyTimes()
 	tested, err := NewComposeService(cli)
 	assert.NilError(t, err)
 	return tested.(*composeService), apiClient
@@ -67,6 +69,23 @@ func emptyLogs() client.ContainerLogsResult {
 	return io.NopCloser(bytes.NewReader(nil))
 }
 
+// expectEmptyOrphanScan sets up the ContainerList expectation for the orphan
+// pre_start cleanup that happens once at the start of every runPreStart call
+// (after the per_replica validation loop). It returns the empty list.
+func expectEmptyOrphanScan(apiClient *mocks.MockAPIClient) *gomock.Call {
+	return apiClient.EXPECT().
+		ContainerList(gomock.Any(), gomock.Any()).
+		Return(client.ContainerListResult{}, nil)
+}
+
+// expectSuccessRemove sets up the ContainerRemove call that runPreStartHook
+// makes after a successful hook run (mirrors old AutoRemove behaviour).
+func expectSuccessRemove(apiClient *mocks.MockAPIClient, hookID string) *gomock.Call {
+	return apiClient.EXPECT().
+		ContainerRemove(gomock.Any(), hookID, client.ContainerRemoveOptions{RemoveVolumes: true}).
+		Return(client.ContainerRemoveResult{}, nil)
+}
+
 func TestPreStart_SuccessTwoHooksInOrder(t *testing.T) {
 	tested, apiClient := newPreStartTestService(t)
 
@@ -81,25 +100,30 @@ func TestPreStart_SuccessTwoHooksInOrder(t *testing.T) {
 	}
 	ctr := container.Summary{ID: "service-ctr-id"}
 
-	// Hook 1: create → wait (subscribe) → logs (subscribe) → start.
+	// Orphan cleanup runs once before the first hook.
+	scan := expectEmptyOrphanScan(apiClient)
+
+	// Hook 1: create → wait (subscribe) → logs (subscribe) → start → remove.
 	create1 := apiClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any()).
-		Return(client.ContainerCreateResult{ID: "hook-1"}, nil)
+		Return(client.ContainerCreateResult{ID: "hook-1"}, nil).After(scan)
 	wait1 := apiClient.EXPECT().ContainerWait(gomock.Any(), "hook-1", gomock.Any()).
 		Return(waitResultExit(0)).After(create1)
 	logs1 := apiClient.EXPECT().ContainerLogs(gomock.Any(), "hook-1", gomock.Any()).
 		Return(emptyLogs(), nil).After(wait1)
 	start1 := apiClient.EXPECT().ContainerStart(gomock.Any(), "hook-1", gomock.Any()).
 		Return(client.ContainerStartResult{}, nil).After(logs1)
+	remove1 := expectSuccessRemove(apiClient, "hook-1").After(start1)
 
-	// Hook 2 is only created after hook 1 has been started (and waited on).
+	// Hook 2 is only created after hook 1 has been removed.
 	create2 := apiClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any()).
-		Return(client.ContainerCreateResult{ID: "hook-2"}, nil).After(start1)
+		Return(client.ContainerCreateResult{ID: "hook-2"}, nil).After(remove1)
 	wait2 := apiClient.EXPECT().ContainerWait(gomock.Any(), "hook-2", gomock.Any()).
 		Return(waitResultExit(0)).After(create2)
 	logs2 := apiClient.EXPECT().ContainerLogs(gomock.Any(), "hook-2", gomock.Any()).
 		Return(emptyLogs(), nil).After(wait2)
-	apiClient.EXPECT().ContainerStart(gomock.Any(), "hook-2", gomock.Any()).
+	start2 := apiClient.EXPECT().ContainerStart(gomock.Any(), "hook-2", gomock.Any()).
 		Return(client.ContainerStartResult{}, nil).After(logs2)
+	expectSuccessRemove(apiClient, "hook-2").After(start2)
 
 	err := tested.runPreStart(t.Context(), project, service, ctr, func(api.ContainerEvent) {})
 	assert.NilError(t, err)
@@ -119,14 +143,16 @@ func TestPreStart_FirstHookFailsStopsExecution(t *testing.T) {
 	}
 	ctr := container.Summary{ID: "service-ctr-id"}
 
+	scan := expectEmptyOrphanScan(apiClient)
 	create1 := apiClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any()).
-		Return(client.ContainerCreateResult{ID: "hook-1"}, nil)
+		Return(client.ContainerCreateResult{ID: "hook-1"}, nil).After(scan)
 	wait1 := apiClient.EXPECT().ContainerWait(gomock.Any(), "hook-1", gomock.Any()).
 		Return(waitResultExit(42)).After(create1)
 	logs1 := apiClient.EXPECT().ContainerLogs(gomock.Any(), "hook-1", gomock.Any()).
 		Return(emptyLogs(), nil).After(wait1)
 	apiClient.EXPECT().ContainerStart(gomock.Any(), "hook-1", gomock.Any()).
 		Return(client.ContainerStartResult{}, nil).After(logs1)
+	// Hook container is retained on failure — no ContainerRemove expected.
 
 	err := tested.runPreStart(t.Context(), project, service, ctr, func(api.ContainerEvent) {})
 	assert.ErrorContains(t, err, `service "web" pre_start[0]`)
@@ -165,17 +191,19 @@ func TestPreStart_ImageFallsBackToBuiltImage(t *testing.T) {
 	ctr := container.Summary{ID: "service-ctr-id"}
 
 	var gotImage string
+	scan := expectEmptyOrphanScan(apiClient)
 	apiClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any()).
 		DoAndReturn(func(_ any, opts client.ContainerCreateOptions) (client.ContainerCreateResult, error) {
 			gotImage = opts.Config.Image
 			return client.ContainerCreateResult{ID: "hook-1"}, nil
-		})
+		}).After(scan)
 	apiClient.EXPECT().ContainerStart(gomock.Any(), "hook-1", gomock.Any()).
 		Return(client.ContainerStartResult{}, nil)
 	apiClient.EXPECT().ContainerLogs(gomock.Any(), "hook-1", gomock.Any()).
 		Return(emptyLogs(), nil)
 	apiClient.EXPECT().ContainerWait(gomock.Any(), "hook-1", gomock.Any()).
 		Return(waitResultExit(0))
+	expectSuccessRemove(apiClient, "hook-1")
 
 	err := tested.runPreStart(t.Context(), project, service, ctr, func(api.ContainerEvent) {})
 	assert.NilError(t, err)
@@ -196,17 +224,19 @@ func TestPreStart_ExplicitHookImageUsed(t *testing.T) {
 	ctr := container.Summary{ID: "service-ctr-id"}
 
 	var gotImage string
+	scan := expectEmptyOrphanScan(apiClient)
 	apiClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any()).
 		DoAndReturn(func(_ any, opts client.ContainerCreateOptions) (client.ContainerCreateResult, error) {
 			gotImage = opts.Config.Image
 			return client.ContainerCreateResult{ID: "hook-1"}, nil
-		})
+		}).After(scan)
 	apiClient.EXPECT().ContainerStart(gomock.Any(), "hook-1", gomock.Any()).
 		Return(client.ContainerStartResult{}, nil)
 	apiClient.EXPECT().ContainerLogs(gomock.Any(), "hook-1", gomock.Any()).
 		Return(emptyLogs(), nil)
 	apiClient.EXPECT().ContainerWait(gomock.Any(), "hook-1", gomock.Any()).
 		Return(waitResultExit(0))
+	expectSuccessRemove(apiClient, "hook-1")
 
 	err := tested.runPreStart(t.Context(), project, service, ctr, func(api.ContainerEvent) {})
 	assert.NilError(t, err)
@@ -228,23 +258,31 @@ func TestPreStart_VolumesFromServiceContainer(t *testing.T) {
 
 	var gotVolumesFrom []string
 	var gotAutoRemove bool
+	var gotLabels map[string]string
+	scan := expectEmptyOrphanScan(apiClient)
 	apiClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any()).
 		DoAndReturn(func(_ any, opts client.ContainerCreateOptions) (client.ContainerCreateResult, error) {
 			gotVolumesFrom = opts.HostConfig.VolumesFrom
 			gotAutoRemove = opts.HostConfig.AutoRemove
+			gotLabels = opts.Config.Labels
 			return client.ContainerCreateResult{ID: "hook-1"}, nil
-		})
+		}).After(scan)
 	apiClient.EXPECT().ContainerStart(gomock.Any(), "hook-1", gomock.Any()).
 		Return(client.ContainerStartResult{}, nil)
 	apiClient.EXPECT().ContainerLogs(gomock.Any(), "hook-1", gomock.Any()).
 		Return(emptyLogs(), nil)
 	apiClient.EXPECT().ContainerWait(gomock.Any(), "hook-1", gomock.Any()).
 		Return(waitResultExit(0))
+	expectSuccessRemove(apiClient, "hook-1")
 
 	err := tested.runPreStart(t.Context(), project, service, ctr, func(api.ContainerEvent) {})
 	assert.NilError(t, err)
 	assert.DeepEqual(t, gotVolumesFrom, []string{"service-ctr-id"})
-	assert.Assert(t, gotAutoRemove)
+	// AutoRemove must be false: the hook container is retained on failure for
+	// post-mortem and explicitly removed on success by runPreStartHook.
+	assert.Assert(t, !gotAutoRemove, "AutoRemove must be false")
+	// HookLabel must be set so orphan cleanup can identify the container.
+	assert.Equal(t, gotLabels[api.HookLabel], preStartHookType)
 }
 
 func TestPreStart_ContainerCreateFailurePropagates(t *testing.T) {
@@ -261,8 +299,9 @@ func TestPreStart_ContainerCreateFailurePropagates(t *testing.T) {
 	}
 	ctr := container.Summary{ID: "service-ctr-id"}
 
+	scan := expectEmptyOrphanScan(apiClient)
 	apiClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any()).
-		Return(client.ContainerCreateResult{}, fmt.Errorf("no such image: missing:latest"))
+		Return(client.ContainerCreateResult{}, fmt.Errorf("no such image: missing:latest")).After(scan)
 
 	err := tested.runPreStart(t.Context(), project, service, ctr, func(api.ContainerEvent) {})
 	assert.ErrorContains(t, err, "no such image")
@@ -281,8 +320,9 @@ func TestPreStart_ContainerStartFailurePropagates(t *testing.T) {
 	}
 	ctr := container.Summary{ID: "service-ctr-id"}
 
+	scan := expectEmptyOrphanScan(apiClient)
 	create1 := apiClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any()).
-		Return(client.ContainerCreateResult{ID: "hook-1"}, nil)
+		Return(client.ContainerCreateResult{ID: "hook-1"}, nil).After(scan)
 	wait1 := apiClient.EXPECT().ContainerWait(gomock.Any(), "hook-1", gomock.Any()).
 		Return(waitResultExit(0)).After(create1)
 	logs1 := apiClient.EXPECT().ContainerLogs(gomock.Any(), "hook-1", gomock.Any()).
@@ -290,8 +330,9 @@ func TestPreStart_ContainerStartFailurePropagates(t *testing.T) {
 	start1 := apiClient.EXPECT().ContainerStart(gomock.Any(), "hook-1", gomock.Any()).
 		Return(client.ContainerStartResult{}, fmt.Errorf("daemon: container start failed")).After(logs1)
 	// AutoRemove never fires when start fails, so the hook must drop the ghost
-	// container explicitly.
-	apiClient.EXPECT().ContainerRemove(gomock.Any(), "hook-1", gomock.Any()).
+	// container explicitly. This is distinct from the success-path removal
+	// (RemoveVolumes:true) — the never-started container has no logs to preserve.
+	apiClient.EXPECT().ContainerRemove(gomock.Any(), "hook-1", client.ContainerRemoveOptions{Force: true, RemoveVolumes: true}).
 		Return(client.ContainerRemoveResult{}, nil).After(start1)
 
 	err := tested.runPreStart(t.Context(), project, service, ctr, func(api.ContainerEvent) {})
@@ -324,14 +365,16 @@ func TestPreStart_WaitResultPreferredOverNilError(t *testing.T) {
 	resultC <- container.WaitResponse{StatusCode: 0}
 	errC <- nil
 
+	scan := expectEmptyOrphanScan(apiClient)
 	apiClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any()).
-		Return(client.ContainerCreateResult{ID: "hook-1"}, nil)
+		Return(client.ContainerCreateResult{ID: "hook-1"}, nil).After(scan)
 	apiClient.EXPECT().ContainerWait(gomock.Any(), "hook-1", gomock.Any()).
 		Return(client.ContainerWaitResult{Result: resultC, Error: errC})
 	apiClient.EXPECT().ContainerLogs(gomock.Any(), "hook-1", gomock.Any()).
 		Return(emptyLogs(), nil)
 	apiClient.EXPECT().ContainerStart(gomock.Any(), "hook-1", gomock.Any()).
 		Return(client.ContainerStartResult{}, nil)
+	expectSuccessRemove(apiClient, "hook-1")
 
 	err := tested.runPreStart(t.Context(), project, service, ctr, func(api.ContainerEvent) {})
 	assert.NilError(t, err)
@@ -399,8 +442,9 @@ func TestPreStart_DetachedModeAttachesLogs(t *testing.T) {
 	}
 	ctr := container.Summary{ID: "service-ctr-id"}
 
+	scan := expectEmptyOrphanScan(apiClient)
 	create1 := apiClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any()).
-		Return(client.ContainerCreateResult{ID: "hook-1"}, nil)
+		Return(client.ContainerCreateResult{ID: "hook-1"}, nil).After(scan)
 	wait1 := apiClient.EXPECT().ContainerWait(gomock.Any(), "hook-1", gomock.Any()).
 		Return(waitResultExit(0)).After(create1)
 	// ContainerLogs MUST be called even with a nil listener.
@@ -408,6 +452,7 @@ func TestPreStart_DetachedModeAttachesLogs(t *testing.T) {
 		Return(emptyLogs(), nil).After(wait1)
 	apiClient.EXPECT().ContainerStart(gomock.Any(), "hook-1", gomock.Any()).
 		Return(client.ContainerStartResult{}, nil).After(logs1)
+	expectSuccessRemove(apiClient, "hook-1")
 
 	err := tested.runPreStart(t.Context(), project, service, ctr, nil)
 	assert.NilError(t, err)
@@ -435,14 +480,16 @@ func TestPreStart_FailureIncludesTail(t *testing.T) {
 		stdcopyFrame(2, "table 'sites' doesn't exist\n")..., // stderr error
 	)
 
+	scan := expectEmptyOrphanScan(apiClient)
 	create1 := apiClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any()).
-		Return(client.ContainerCreateResult{ID: "hook-1"}, nil)
+		Return(client.ContainerCreateResult{ID: "hook-1"}, nil).After(scan)
 	wait1 := apiClient.EXPECT().ContainerWait(gomock.Any(), "hook-1", gomock.Any()).
 		Return(waitResultExit(1)).After(create1)
 	logs1 := apiClient.EXPECT().ContainerLogs(gomock.Any(), "hook-1", gomock.Any()).
 		Return(io.NopCloser(bytes.NewReader(logContent)), nil).After(wait1)
 	apiClient.EXPECT().ContainerStart(gomock.Any(), "hook-1", gomock.Any()).
 		Return(client.ContainerStartResult{}, nil).After(logs1)
+	// Hook container is retained on failure — no ContainerRemove expected.
 
 	err := tested.runPreStart(t.Context(), project, service, ctr, nil)
 	assert.Assert(t, err != nil)
@@ -450,3 +497,685 @@ func TestPreStart_FailureIncludesTail(t *testing.T) {
 	// Stderr content must be in the error (stderr bias).
 	assert.ErrorContains(t, err, "doesn't exist")
 }
+
+// ---------------------------------------------------------------------------
+// Feature tests: pre_start container retention on failure
+// ---------------------------------------------------------------------------
+
+// TestPreStart_SuccessRemovesContainer verifies that a successful pre_start hook
+// triggers an explicit ContainerRemove (with RemoveVolumes: true to mirror the
+// old AutoRemove behaviour) and that AutoRemove is false at create time.
+func TestPreStart_SuccessRemovesContainer(t *testing.T) {
+	tested, apiClient := newPreStartTestService(t)
+
+	project := &types.Project{Name: "proj"}
+	service := types.ServiceConfig{
+		Name:  "web",
+		Image: "alpine",
+		PreStart: []types.ServiceHook{
+			{Image: "alpine", Command: types.ShellCommand{"true"}},
+		},
+	}
+	ctr := container.Summary{ID: "svc-ctr"}
+
+	var gotAutoRemove bool
+	scan := expectEmptyOrphanScan(apiClient)
+	apiClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any()).
+		DoAndReturn(func(_ any, opts client.ContainerCreateOptions) (client.ContainerCreateResult, error) {
+			gotAutoRemove = opts.HostConfig.AutoRemove
+			return client.ContainerCreateResult{ID: "hook-1"}, nil
+		}).After(scan)
+	apiClient.EXPECT().ContainerWait(gomock.Any(), "hook-1", gomock.Any()).
+		Return(waitResultExit(0))
+	apiClient.EXPECT().ContainerLogs(gomock.Any(), "hook-1", gomock.Any()).
+		Return(emptyLogs(), nil)
+	apiClient.EXPECT().ContainerStart(gomock.Any(), "hook-1", gomock.Any()).
+		Return(client.ContainerStartResult{}, nil)
+	// Success path: explicit remove with RemoveVolumes: true.
+	apiClient.EXPECT().
+		ContainerRemove(gomock.Any(), "hook-1", client.ContainerRemoveOptions{RemoveVolumes: true}).
+		Return(client.ContainerRemoveResult{}, nil)
+
+	err := tested.runPreStart(t.Context(), project, service, ctr, nil)
+	assert.NilError(t, err)
+	assert.Assert(t, !gotAutoRemove, "AutoRemove must be false; retention is managed explicitly")
+}
+
+// TestPreStart_FailureRetainsContainer verifies that a pre_start hook that exits
+// with a non-zero code does NOT trigger ContainerRemove — the container is kept
+// for post-mortem inspection.
+func TestPreStart_FailureRetainsContainer(t *testing.T) {
+	tested, apiClient := newPreStartTestService(t)
+
+	project := &types.Project{Name: "proj"}
+	service := types.ServiceConfig{
+		Name:  "web",
+		Image: "alpine",
+		PreStart: []types.ServiceHook{
+			{Image: "alpine", Command: types.ShellCommand{"migrate"}},
+		},
+	}
+	ctr := container.Summary{ID: "svc-ctr"}
+
+	logContent := stdcopyFrame(2, "migration failed: table missing\n")
+
+	scan := expectEmptyOrphanScan(apiClient)
+	apiClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any()).
+		Return(client.ContainerCreateResult{ID: "hook-1"}, nil).After(scan)
+	apiClient.EXPECT().ContainerWait(gomock.Any(), "hook-1", gomock.Any()).
+		Return(waitResultExit(1))
+	apiClient.EXPECT().ContainerLogs(gomock.Any(), "hook-1", gomock.Any()).
+		Return(io.NopCloser(bytes.NewReader(logContent)), nil)
+	apiClient.EXPECT().ContainerStart(gomock.Any(), "hook-1", gomock.Any()).
+		Return(client.ContainerStartResult{}, nil)
+	// No ContainerRemove expectation: gomock fails on unexpected calls.
+
+	err := tested.runPreStart(t.Context(), project, service, ctr, nil)
+	assert.ErrorContains(t, err, "pre_start[0]")
+	assert.ErrorContains(t, err, "table missing")
+	// Short container ID must appear in the error so the operator can run
+	// `docker logs hook-1` immediately.
+	assert.ErrorContains(t, err, "hook-1")
+}
+
+// TestPreStart_CancellationRemovesContainer verifies that when the context is
+// cancelled while a hook is running the container is removed (not retained) and
+// the error is exactly ctx.Err() with no tail decoration.
+func TestPreStart_CancellationRemovesContainer(t *testing.T) {
+	tested, apiClient := newPreStartTestService(t)
+
+	ctx, cancel := context.WithCancel(t.Context())
+
+	project := &types.Project{Name: "proj"}
+	service := types.ServiceConfig{
+		Name:  "web",
+		Image: "alpine",
+		PreStart: []types.ServiceHook{
+			{Image: "alpine", Command: types.ShellCommand{"long-running-op"}},
+		},
+	}
+	ctr := container.Summary{ID: "svc-ctr"}
+
+	scan := expectEmptyOrphanScan(apiClient)
+	apiClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any()).
+		Return(client.ContainerCreateResult{ID: "hook-cancel-123"}, nil).After(scan)
+	// ContainerWait channel: cancel the context before delivering any result so
+	// waitPreStart returns ctx.Err().
+	resultC := make(chan container.WaitResponse)
+	errC := make(chan error)
+	apiClient.EXPECT().ContainerWait(gomock.Any(), "hook-cancel-123", gomock.Any()).
+		Return(client.ContainerWaitResult{Result: resultC, Error: errC})
+	apiClient.EXPECT().ContainerLogs(gomock.Any(), "hook-cancel-123", gomock.Any()).
+		Return(emptyLogs(), nil)
+	apiClient.EXPECT().ContainerStart(gomock.Any(), "hook-cancel-123", gomock.Any()).
+		Return(client.ContainerStartResult{}, nil)
+	// On cancellation the container must be removed, not retained.
+	apiClient.EXPECT().
+		ContainerRemove(gomock.Any(), "hook-cancel-123", client.ContainerRemoveOptions{Force: true, RemoveVolumes: true}).
+		Return(client.ContainerRemoveResult{}, nil)
+
+	errCh := make(chan error, 1)
+	go func() {
+		errCh <- tested.runPreStart(ctx, project, service, ctr, nil)
+	}()
+	// Cancel after the hook container has started.
+	cancel()
+
+	err := <-errCh
+	assert.ErrorIs(t, err, context.Canceled, "must return ctx.Err() on cancellation")
+	// The error must NOT include "retained" — it should be the raw context error.
+	assert.Assert(t, !strings.Contains(err.Error(), "retained"), "cancelled hook must not be retained; got: %s", err)
+}
+
+// TestPreStart_RemovesOrphanBeforeRun verifies that a hook container left behind
+// by a previous failed run is force-removed before the new container is created.
+func TestPreStart_RemovesOrphanBeforeRun(t *testing.T) {
+	tested, apiClient := newPreStartTestService(t)
+
+	project := &types.Project{Name: "proj"}
+	service := types.ServiceConfig{
+		Name:  "web",
+		Image: "alpine",
+		PreStart: []types.ServiceHook{
+			{Image: "alpine", Command: types.ShellCommand{"migrate"}},
+		},
+	}
+	ctr := container.Summary{ID: "svc-ctr"}
+
+	// ContainerList returns the stale orphan from a previous run.
+	orphan := container.Summary{ID: "orphan-hook-123", State: "exited"}
+	scan := apiClient.EXPECT().ContainerList(gomock.Any(), gomock.Any()).
+		Return(client.ContainerListResult{Items: []container.Summary{orphan}}, nil)
+	// Orphan must be removed before the new hook container is created.
+	removeOrphan := apiClient.EXPECT().
+		ContainerRemove(gomock.Any(), "orphan-hook-123", client.ContainerRemoveOptions{Force: true, RemoveVolumes: true}).
+		Return(client.ContainerRemoveResult{}, nil).After(scan)
+	create1 := apiClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any()).
+		Return(client.ContainerCreateResult{ID: "hook-1"}, nil).After(removeOrphan)
+	apiClient.EXPECT().ContainerWait(gomock.Any(), "hook-1", gomock.Any()).
+		Return(waitResultExit(0)).After(create1)
+	apiClient.EXPECT().ContainerLogs(gomock.Any(), "hook-1", gomock.Any()).
+		Return(emptyLogs(), nil)
+	apiClient.EXPECT().ContainerStart(gomock.Any(), "hook-1", gomock.Any()).
+		Return(client.ContainerStartResult{}, nil)
+	expectSuccessRemove(apiClient, "hook-1")
+
+	err := tested.runPreStart(t.Context(), project, service, ctr, nil)
+	assert.NilError(t, err)
+}
+
+// TestPreStart_SuccessRemoveFailureIsNonFatal verifies that a ContainerRemove
+// failure on the success path is logged but does not fail runPreStart.
+func TestPreStart_SuccessRemoveFailureIsNonFatal(t *testing.T) {
+	tested, apiClient := newPreStartTestService(t)
+
+	project := &types.Project{Name: "proj"}
+	service := types.ServiceConfig{
+		Name:  "web",
+		Image: "alpine",
+		PreStart: []types.ServiceHook{
+			{Image: "alpine", Command: types.ShellCommand{"true"}},
+		},
+	}
+	ctr := container.Summary{ID: "svc-ctr"}
+
+	scan := expectEmptyOrphanScan(apiClient)
+	apiClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any()).
+		Return(client.ContainerCreateResult{ID: "hook-1"}, nil).After(scan)
+	apiClient.EXPECT().ContainerWait(gomock.Any(), "hook-1", gomock.Any()).
+		Return(waitResultExit(0))
+	apiClient.EXPECT().ContainerLogs(gomock.Any(), "hook-1", gomock.Any()).
+		Return(emptyLogs(), nil)
+	apiClient.EXPECT().ContainerStart(gomock.Any(), "hook-1", gomock.Any()).
+		Return(client.ContainerStartResult{}, nil)
+	// Simulate a removal failure.
+	apiClient.EXPECT().
+		ContainerRemove(gomock.Any(), "hook-1", client.ContainerRemoveOptions{RemoveVolumes: true}).
+		Return(client.ContainerRemoveResult{}, fmt.Errorf("already removed"))
+
+	// The hook succeeded; the service must start even if removal failed.
+	err := tested.runPreStart(t.Context(), project, service, ctr, nil)
+	assert.NilError(t, err)
+}
+
+// ---------------------------------------------------------------------------
+// Coverage-gap tests: previously uncovered branches
+// ---------------------------------------------------------------------------
+
+// TestLowestNumberedContainer verifies that lowestNumberedContainer always
+// picks the replica with the smallest ContainerNumberLabel value.
+func TestLowestNumberedContainer(t *testing.T) {
+	makeCtr := func(id, num string) container.Summary {
+		return container.Summary{ID: id, Labels: map[string]string{
+			api.ContainerNumberLabel: num,
+		}}
+	}
+	tests := []struct {
+		name       string
+		containers Containers
+		wantID     string
+	}{
+		{"single", Containers{makeCtr("a", "1")}, "a"},
+		{"ascending", Containers{makeCtr("a", "1"), makeCtr("b", "2"), makeCtr("c", "3")}, "a"},
+		{"descending", Containers{makeCtr("a", "3"), makeCtr("b", "2"), makeCtr("c", "1")}, "c"},
+		{"unordered", Containers{makeCtr("a", "5"), makeCtr("b", "1"), makeCtr("c", "3")}, "b"},
+		{"missing_label", Containers{makeCtr("x", ""), makeCtr("y", "1")}, "x"}, // strconv.Atoi("") = 0 = lowest
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			got := lowestNumberedContainer(tc.containers)
+			assert.Equal(t, got.ID, tc.wantID)
+		})
+	}
+}
+
+// TestPreStart_WaitCancelled covers the ctx.Done() branch in waitPreStart:
+// when the context is already canceled the function must return ctx.Err()
+// without blocking on the result or error channels.
+func TestPreStart_WaitCancelled(t *testing.T) {
+	ctx, cancel := context.WithCancel(t.Context())
+	cancel() // already canceled
+
+	resultC := make(chan container.WaitResponse) // never sends
+	errC := make(chan error)                     // never sends
+	waitRes := client.ContainerWaitResult{Result: resultC, Error: errC}
+
+	err := waitPreStart(ctx, "web", 0, waitRes)
+	assert.ErrorIs(t, err, context.Canceled)
+}
+
+// TestPreStart_ResultErrWaitError covers the preStartResultErr branch where
+// the WaitResponse carries a daemon-reported error message.
+func TestPreStart_ResultErrWaitError(t *testing.T) {
+	res := container.WaitResponse{
+		Error: &container.WaitExitError{Message: "oom killed"},
+	}
+	err := preStartResultErr("web", 0, res)
+	assert.ErrorContains(t, err, "wait error")
+	assert.ErrorContains(t, err, "oom killed")
+}
+
+// TestPreStart_StreamLogsError_NilListener covers the ContainerLogs-failure
+// branch in streamPreStartLogs when the caller passes no listener.
+// The hook must still start and succeed; the missing log stream is non-fatal.
+func TestPreStart_StreamLogsError_NilListener(t *testing.T) {
+	tested, apiClient := newPreStartTestService(t)
+
+	project := &types.Project{Name: "proj"}
+	service := types.ServiceConfig{
+		Name:  "web",
+		Image: "alpine",
+		PreStart: []types.ServiceHook{
+			{Image: "alpine", Command: types.ShellCommand{"true"}},
+		},
+	}
+	ctr := container.Summary{ID: "svc-ctr"}
+
+	scan := expectEmptyOrphanScan(apiClient)
+	apiClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any()).
+		Return(client.ContainerCreateResult{ID: "hook-1"}, nil).After(scan)
+	apiClient.EXPECT().ContainerWait(gomock.Any(), "hook-1", gomock.Any()).
+		Return(waitResultExit(0))
+	// ContainerLogs fails; nil listener → no warning event, done closed immediately.
+	apiClient.EXPECT().ContainerLogs(gomock.Any(), "hook-1", gomock.Any()).
+		Return(nil, fmt.Errorf("logs: connection refused"))
+	apiClient.EXPECT().ContainerStart(gomock.Any(), "hook-1", gomock.Any()).
+		Return(client.ContainerStartResult{}, nil)
+	expectSuccessRemove(apiClient, "hook-1")
+
+	err := tested.runPreStart(t.Context(), project, service, ctr, nil)
+	assert.NilError(t, err)
+}
+
+// TestPreStart_StreamLogsError_WithListener covers the ContainerLogs-failure
+// branch in streamPreStartLogs when a listener IS present: a warning event
+// must be emitted on the listener.
+func TestPreStart_StreamLogsError_WithListener(t *testing.T) {
+	tested, apiClient := newPreStartTestService(t)
+
+	project := &types.Project{Name: "proj"}
+	service := types.ServiceConfig{
+		Name:  "web",
+		Image: "alpine",
+		PreStart: []types.ServiceHook{
+			{Image: "alpine", Command: types.ShellCommand{"true"}},
+		},
+	}
+	ctr := container.Summary{ID: "svc-ctr"}
+
+	scan := expectEmptyOrphanScan(apiClient)
+	apiClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any()).
+		Return(client.ContainerCreateResult{ID: "hook-1"}, nil).After(scan)
+	apiClient.EXPECT().ContainerWait(gomock.Any(), "hook-1", gomock.Any()).
+		Return(waitResultExit(0))
+	apiClient.EXPECT().ContainerLogs(gomock.Any(), "hook-1", gomock.Any()).
+		Return(nil, fmt.Errorf("logs: daemon unavailable"))
+	apiClient.EXPECT().ContainerStart(gomock.Any(), "hook-1", gomock.Any()).
+		Return(client.ContainerStartResult{}, nil)
+	expectSuccessRemove(apiClient, "hook-1")
+
+	var gotWarning string
+	listener := func(ev api.ContainerEvent) {
+		gotWarning = ev.Line
+	}
+	err := tested.runPreStart(t.Context(), project, service, ctr, listener)
+	assert.NilError(t, err)
+	assert.Assert(t, gotWarning != "", "listener must receive a warning when ContainerLogs fails")
+	assert.Assert(t, bytes.Contains([]byte(gotWarning), []byte("warning")), "expected 'warning' in: %q", gotWarning)
+}
+
+// TestPreStart_OldAPIVersion covers the versions.LessThan(apiVersion, "1.44")
+// branch in createPreStartContainer: on a pre-1.44 daemon the extra-networks
+// path runs via connectPreStartExtraNetworks. With only one (primary) network
+// no NetworkConnect call is issued and the hook succeeds normally.
+func TestPreStart_OldAPIVersion(t *testing.T) {
+	tested, apiClient := newPreStartTestServiceWithVersion(t, "1.43")
+
+	project := &types.Project{
+		Name: "proj",
+		Networks: types.Networks{
+			"default": {Name: "proj_default"},
+		},
+	}
+	service := types.ServiceConfig{
+		Name:  "web",
+		Image: "alpine",
+		Networks: map[string]*types.ServiceNetworkConfig{
+			"default": nil,
+		},
+		PreStart: []types.ServiceHook{
+			{Image: "alpine", Command: types.ShellCommand{"true"}},
+		},
+	}
+	ctr := container.Summary{ID: "svc-ctr"}
+
+	scan := expectEmptyOrphanScan(apiClient)
+	apiClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any()).
+		Return(client.ContainerCreateResult{ID: "hook-1"}, nil).After(scan)
+	apiClient.EXPECT().ContainerWait(gomock.Any(), "hook-1", gomock.Any()).
+		Return(waitResultExit(0))
+	apiClient.EXPECT().ContainerLogs(gomock.Any(), "hook-1", gomock.Any()).
+		Return(emptyLogs(), nil)
+	apiClient.EXPECT().ContainerStart(gomock.Any(), "hook-1", gomock.Any()).
+		Return(client.ContainerStartResult{}, nil)
+	// Single network = primary only; no NetworkConnect expected.
+	expectSuccessRemove(apiClient, "hook-1")
+
+	err := tested.runPreStart(t.Context(), project, service, ctr, nil)
+	assert.NilError(t, err)
+}
+
+// TestPreStart_ConnectExtraNetworksSuccess covers connectPreStartExtraNetworks
+// when a secondary network is present: NetworkConnect must be called for it.
+func TestPreStart_ConnectExtraNetworksSuccess(t *testing.T) {
+	tested, apiClient := newPreStartTestService(t)
+
+	project := &types.Project{
+		Name: "proj",
+		Networks: types.Networks{
+			"default": {Name: "proj_default"},
+			"extra":   {Name: "proj_extra"},
+		},
+	}
+	service := types.ServiceConfig{
+		Name:  "web",
+		Image: "alpine",
+		Networks: map[string]*types.ServiceNetworkConfig{
+			"default": nil,
+			"extra":   nil,
+		},
+	}
+
+	apiClient.EXPECT().
+		NetworkConnect(gomock.Any(), "proj_extra", gomock.Any()).
+		Return(client.NetworkConnectResult{}, nil)
+
+	err := tested.connectPreStartExtraNetworks(t.Context(), project, service, "ctr-id", "proj_default")
+	assert.NilError(t, err)
+}
+
+// TestPreStart_ConnectExtraNetworksFails covers the NetworkConnect error branch
+// in connectPreStartExtraNetworks.
+func TestPreStart_ConnectExtraNetworksFails(t *testing.T) {
+	tested, apiClient := newPreStartTestService(t)
+
+	project := &types.Project{
+		Name: "proj",
+		Networks: types.Networks{
+			"default": {Name: "proj_default"},
+			"extra":   {Name: "proj_extra"},
+		},
+	}
+	service := types.ServiceConfig{
+		Name:  "web",
+		Image: "alpine",
+		Networks: map[string]*types.ServiceNetworkConfig{
+			"default": nil,
+			"extra":   nil,
+		},
+	}
+
+	apiClient.EXPECT().
+		NetworkConnect(gomock.Any(), "proj_extra", gomock.Any()).
+		Return(client.NetworkConnectResult{}, fmt.Errorf("network not found"))
+
+	err := tested.connectPreStartExtraNetworks(t.Context(), project, service, "ctr-id", "proj_default")
+	assert.ErrorContains(t, err, "network not found")
+}
+
+// TestPreStart_ContainerStartFailureAndRemoveFails covers the Warnf path in
+// runPreStartHook when ContainerStart fails AND the subsequent ContainerRemove
+// also fails (the orphan is unremovable but the caller still gets the start error).
+func TestPreStart_ContainerStartFailureAndRemoveFails(t *testing.T) {
+	tested, apiClient := newPreStartTestService(t)
+
+	project := &types.Project{Name: "proj"}
+	service := types.ServiceConfig{
+		Name:  "web",
+		Image: "alpine",
+		PreStart: []types.ServiceHook{
+			{Image: "alpine", Command: types.ShellCommand{"true"}},
+		},
+	}
+	ctr := container.Summary{ID: "svc-ctr"}
+
+	scan := expectEmptyOrphanScan(apiClient)
+	create1 := apiClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any()).
+		Return(client.ContainerCreateResult{ID: "hook-1"}, nil).After(scan)
+	apiClient.EXPECT().ContainerWait(gomock.Any(), "hook-1", gomock.Any()).
+		Return(waitResultExit(0)).After(create1)
+	logs1 := apiClient.EXPECT().ContainerLogs(gomock.Any(), "hook-1", gomock.Any()).
+		Return(emptyLogs(), nil)
+	start1 := apiClient.EXPECT().ContainerStart(gomock.Any(), "hook-1", gomock.Any()).
+		Return(client.ContainerStartResult{}, fmt.Errorf("start failed")).After(logs1)
+	// Both the start AND the cleanup removal fail.
+	apiClient.EXPECT().ContainerRemove(gomock.Any(), "hook-1", client.ContainerRemoveOptions{Force: true, RemoveVolumes: true}).
+		Return(client.ContainerRemoveResult{}, fmt.Errorf("removal failed")).After(start1)
+
+	err := tested.runPreStart(t.Context(), project, service, ctr, nil)
+	// The original start error must be returned, not the removal error.
+	assert.ErrorContains(t, err, "start failed")
+}
+
+// TestPreStart_OrphanScanFails verifies that when ContainerList fails during
+// orphan cleanup the Warnf path in runPreStart is hit, but execution continues
+// and the hook runs successfully.
+func TestPreStart_OrphanScanFails(t *testing.T) {
+	tested, apiClient := newPreStartTestService(t)
+
+	project := &types.Project{Name: "proj"}
+	service := types.ServiceConfig{
+		Name:  "web",
+		Image: "alpine",
+		PreStart: []types.ServiceHook{
+			{Image: "alpine", Command: types.ShellCommand{"true"}},
+		},
+	}
+	ctr := container.Summary{ID: "svc-ctr"}
+
+	// ContainerList fails → Warnf in runPreStart; hook still proceeds.
+	apiClient.EXPECT().ContainerList(gomock.Any(), gomock.Any()).
+		Return(client.ContainerListResult{}, fmt.Errorf("daemon unavailable"))
+	apiClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any()).
+		Return(client.ContainerCreateResult{ID: "hook-1"}, nil)
+	apiClient.EXPECT().ContainerWait(gomock.Any(), "hook-1", gomock.Any()).
+		Return(waitResultExit(0))
+	apiClient.EXPECT().ContainerLogs(gomock.Any(), "hook-1", gomock.Any()).
+		Return(emptyLogs(), nil)
+	apiClient.EXPECT().ContainerStart(gomock.Any(), "hook-1", gomock.Any()).
+		Return(client.ContainerStartResult{}, nil)
+	expectSuccessRemove(apiClient, "hook-1")
+
+	err := tested.runPreStart(t.Context(), project, service, ctr, nil)
+	assert.NilError(t, err)
+}
+
+// TestPreStart_OrphanRemovalFails verifies the Warnf path in
+// removeOrphanPreStartContainers when an individual stale container cannot be
+// removed. The hook must still proceed normally.
+func TestPreStart_OrphanRemovalFails(t *testing.T) {
+	tested, apiClient := newPreStartTestService(t)
+
+	project := &types.Project{Name: "proj"}
+	service := types.ServiceConfig{
+		Name:  "web",
+		Image: "alpine",
+		PreStart: []types.ServiceHook{
+			{Image: "alpine", Command: types.ShellCommand{"true"}},
+		},
+	}
+	ctr := container.Summary{ID: "svc-ctr"}
+
+	orphan := container.Summary{ID: "orphan-123"}
+	// ContainerList finds the orphan; its removal fails → Warnf then continue.
+	apiClient.EXPECT().ContainerList(gomock.Any(), gomock.Any()).
+		Return(client.ContainerListResult{Items: []container.Summary{orphan}}, nil)
+	apiClient.EXPECT().
+		ContainerRemove(gomock.Any(), "orphan-123", client.ContainerRemoveOptions{Force: true, RemoveVolumes: true}).
+		Return(client.ContainerRemoveResult{}, fmt.Errorf("permission denied"))
+	// Hook still runs and succeeds.
+	apiClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any()).
+		Return(client.ContainerCreateResult{ID: "hook-1"}, nil)
+	apiClient.EXPECT().ContainerWait(gomock.Any(), "hook-1", gomock.Any()).
+		Return(waitResultExit(0))
+	apiClient.EXPECT().ContainerLogs(gomock.Any(), "hook-1", gomock.Any()).
+		Return(emptyLogs(), nil)
+	apiClient.EXPECT().ContainerStart(gomock.Any(), "hook-1", gomock.Any()).
+		Return(client.ContainerStartResult{}, nil)
+	expectSuccessRemove(apiClient, "hook-1")
+
+	err := tested.runPreStart(t.Context(), project, service, ctr, nil)
+	assert.NilError(t, err)
+}
+
+// TestPreStart_OldAPINetworkConnectFails covers the createPreStartContainer path
+// for API < 1.44 with a secondary network: when NetworkConnect fails the function
+// must clean up the created-but-never-started container and return the error.
+func TestPreStart_OldAPINetworkConnectFails(t *testing.T) {
+	tested, apiClient := newPreStartTestServiceWithVersion(t, "1.43")
+
+	project := &types.Project{
+		Name: "proj",
+		Networks: types.Networks{
+			"default": {Name: "proj_default"},
+			"extra":   {Name: "proj_extra"},
+		},
+	}
+	service := types.ServiceConfig{
+		Name:  "web",
+		Image: "alpine",
+		Networks: map[string]*types.ServiceNetworkConfig{
+			"default": nil,
+			"extra":   nil,
+		},
+		PreStart: []types.ServiceHook{
+			{Image: "alpine", Command: types.ShellCommand{"true"}},
+		},
+	}
+	ctr := container.Summary{ID: "svc-ctr"}
+
+	scan := expectEmptyOrphanScan(apiClient)
+	apiClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any()).
+		Return(client.ContainerCreateResult{ID: "hook-1"}, nil).After(scan)
+	// NetworkConnect for the secondary network fails.
+	apiClient.EXPECT().NetworkConnect(gomock.Any(), "proj_extra", gomock.Any()).
+		Return(client.NetworkConnectResult{}, fmt.Errorf("network not found"))
+	// The never-started container is cleaned up (Force:true).
+	apiClient.EXPECT().
+		ContainerRemove(gomock.Any(), "hook-1", client.ContainerRemoveOptions{Force: true, RemoveVolumes: true}).
+		Return(client.ContainerRemoveResult{}, nil)
+
+	err := tested.runPreStart(t.Context(), project, service, ctr, nil)
+	assert.ErrorContains(t, err, "network not found")
+}
+
+// TestPreStart_OldAPINetworkConnectAndRemoveFails covers the Warnf path inside
+// createPreStartContainer when both NetworkConnect and the cleanup ContainerRemove fail.
+func TestPreStart_OldAPINetworkConnectAndRemoveFails(t *testing.T) {
+	tested, apiClient := newPreStartTestServiceWithVersion(t, "1.43")
+
+	project := &types.Project{
+		Name: "proj",
+		Networks: types.Networks{
+			"default": {Name: "proj_default"},
+			"extra":   {Name: "proj_extra"},
+		},
+	}
+	service := types.ServiceConfig{
+		Name:  "web",
+		Image: "alpine",
+		Networks: map[string]*types.ServiceNetworkConfig{
+			"default": nil,
+			"extra":   nil,
+		},
+		PreStart: []types.ServiceHook{
+			{Image: "alpine", Command: types.ShellCommand{"true"}},
+		},
+	}
+	ctr := container.Summary{ID: "svc-ctr"}
+
+	scan := expectEmptyOrphanScan(apiClient)
+	apiClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any()).
+		Return(client.ContainerCreateResult{ID: "hook-1"}, nil).After(scan)
+	apiClient.EXPECT().NetworkConnect(gomock.Any(), "proj_extra", gomock.Any()).
+		Return(client.NetworkConnectResult{}, fmt.Errorf("network not found"))
+	// Cleanup removal also fails → Warnf; original error is still returned.
+	apiClient.EXPECT().
+		ContainerRemove(gomock.Any(), "hook-1", client.ContainerRemoveOptions{Force: true, RemoveVolumes: true}).
+		Return(client.ContainerRemoveResult{}, fmt.Errorf("removal also failed"))
+
+	err := tested.runPreStart(t.Context(), project, service, ctr, nil)
+	assert.ErrorContains(t, err, "network not found")
+}
+
+// TestPreStart_RuntimeAPIVersionError covers the early-return in
+// createPreStartContainer when RuntimeAPIVersion fails (Ping returns an error).
+func TestPreStart_RuntimeAPIVersionError(t *testing.T) {
+	ignoreExisting := goleak.IgnoreCurrent()
+	t.Cleanup(func() { goleak.VerifyNone(t, ignoreExisting) })
+
+	mockCtrl := gomock.NewController(t)
+	t.Cleanup(mockCtrl.Finish)
+	apiClient := mocks.NewMockAPIClient(mockCtrl)
+	cli := mocks.NewMockCli(mockCtrl)
+	cli.EXPECT().Client().Return(apiClient).AnyTimes()
+	tested, err := NewComposeService(cli)
+	assert.NilError(t, err)
+	s := tested.(*composeService)
+
+	project := &types.Project{Name: "proj"}
+	service := types.ServiceConfig{
+		Name:  "web",
+		Image: "alpine",
+		PreStart: []types.ServiceHook{
+			{Image: "alpine", Command: types.ShellCommand{"true"}},
+		},
+	}
+	ctr := container.Summary{ID: "svc-ctr"}
+
+	// Orphan scan succeeds (ContainerList does not need Ping).
+	apiClient.EXPECT().ContainerList(gomock.Any(), gomock.Any()).
+		Return(client.ContainerListResult{}, nil)
+	// createPreStartContainer calls RuntimeAPIVersion → Ping fails.
+	apiClient.EXPECT().
+		Ping(gomock.Any(), client.PingOptions{NegotiateAPIVersion: true}).
+		Return(client.PingResult{}, fmt.Errorf("daemon unreachable"))
+
+	err = s.runPreStart(t.Context(), project, service, ctr, nil)
+	assert.ErrorContains(t, err, "daemon unreachable")
+}
+
+// TestPreStart_FailureStdoutOnlyTail covers the stdout-fallback branch in the
+// getTail closure inside streamPreStartLogs: when stderr is empty but stdout has
+// content, getTail must return the stdout content.
+func TestPreStart_FailureStdoutOnlyTail(t *testing.T) {
+	tested, apiClient := newPreStartTestService(t)
+
+	project := &types.Project{Name: "proj"}
+	service := types.ServiceConfig{
+		Name:  "db",
+		Image: "postgres",
+		PreStart: []types.ServiceHook{
+			{Image: "postgres", Command: types.ShellCommand{"migrate"}},
+		},
+	}
+	ctr := container.Summary{ID: "svc-ctr"}
+
+	// stdout-only content: stderr frame absent so getTail falls back to stdout.
+	logContent := stdcopyFrame(1, "migration failed: schema mismatch\n")
+
+	scan := expectEmptyOrphanScan(apiClient)
+	apiClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any()).
+		Return(client.ContainerCreateResult{ID: "hook-1"}, nil).After(scan)
+	apiClient.EXPECT().ContainerWait(gomock.Any(), "hook-1", gomock.Any()).
+		Return(waitResultExit(1))
+	apiClient.EXPECT().ContainerLogs(gomock.Any(), "hook-1", gomock.Any()).
+		Return(io.NopCloser(bytes.NewReader(logContent)), nil)
+	apiClient.EXPECT().ContainerStart(gomock.Any(), "hook-1", gomock.Any()).
+		Return(client.ContainerStartResult{}, nil)
+
+	err := tested.runPreStart(t.Context(), project, service, ctr, nil)
+	assert.ErrorContains(t, err, "pre_start[0]")
+	// Stdout fallback: no stderr → stdout content appears in the error.
+	assert.ErrorContains(t, err, "schema mismatch")
+}