fix: a failed pre_start/start no longer lets its dependent run

A node's done-channel closes strictly before errgroup cancels its
derived context on that same error - the close happens synchronously
inside the failing node's goroutine, ctx cancellation only after it
returns. A dependent blocked in run()'s
select { case <-done[dep.ID]: ...; case <-ctx.Done(): ... } therefore
always sees its dependency's done-channel ready first, regardless of
whether that dependency failed for a genuine reason or was cancelled.

execCreateHookContainer already guarded against exactly this. The two
other new dispatch functions from the previous commit did not:
execStartContainer's enriched branch could call ContainerStart right
after its OpRunPreStart failed for real (e.g. "no hook runner
container found"), contradicting pre_start.go's documented guarantee
that a non-zero hook exit gates service start - and symmetrically for
execRunPostStart after a failed OpStartContainer.

Both now bail out on ctx.Err() first, mirroring the existing guard.
Pinned by a regression test reproducing the race deterministically: no
ContainerStart expectation is registered, so the unguarded code calls
it from inside the errgroup worker goroutine, where gomock's missing-
expectation failure hangs the test binary instead of failing it
cleanly - a stronger tell than a plain assertion failure would have
been.

Signed-off-by: Nicolas De Loof <[email protected]>

pkg/compose/executor_ops.go

@@ -144,6 +144,14 @@ func (exec *planExecutor) execStartContainer(ctx context.Context, op Operation)
 		return err
 	}
 
+	// A dependency's done-channel also closes on failure: when a preceding
+	// node of this replica's chain (a wait, pre_start) failed and canceled
+	// the run, bail out on the cancellation instead of starting a container
+	// whose pre_start hook just failed — same race as execCreateHookContainer.
+	if err := ctx.Err(); err != nil {
+		return err
+	}
+
 	id, err := exec.resolveContainerID(op)
 	if err != nil {
 		return err
@@ -262,6 +270,14 @@ func (exec *planExecutor) execRunPreStart(ctx context.Context, op Operation) err
 // execRunPostStart runs the service's post_start hooks against the replica
 // the start chain just brought up.
 func (exec *planExecutor) execRunPostStart(ctx context.Context, op Operation) error {
+	// Same race as execCreateHookContainer/execStartContainer: a failed
+	// StartContainer's done-channel closes before errgroup's ctx is
+	// canceled, so this must not run post_start against a container that
+	// never actually started.
+	if err := ctx.Err(); err != nil {
+		return err
+	}
+
 	ctr, err := exec.resolveContainerSummary(op)
 	if err != nil {
 		return err

pkg/compose/executor_start_test.go

@@ -435,3 +435,74 @@ func TestExecutePlanMissingRequiredDependencyFailsSilently(t *testing.T) {
 	assert.ErrorContains(t, err, "missing dependency db")
 	assert.Equal(t, len(recorder.byID), 0, "a missing-dependency failure must not emit a spurious event")
 }
+
+// TestExecutePlanFailedPreStartGatesStart is a regression test for a race in
+// run()'s DAG walk: a node's done-channel closes (unblocking its dependents)
+// strictly before errgroup cancels its derived context on that same error —
+// the close happens synchronously inside the failing node's goroutine, ctx
+// cancellation only after it returns. A dependent blocked in run()'s
+// `select { case <-done[dep.ID]: ...; case <-ctx.Done(): ... }` therefore
+// always sees its dependency's done-channel ready first, deterministically,
+// regardless of whether that dependency failed for a genuine reason or was
+// itself cancelled.
+//
+// execCreateHookContainer already guards against this with a leading
+// ctx.Err() check; this test pins the same guard on execStartContainer's
+// enriched branch: when OpRunPreStart fails for a real reason (here: no hook
+// runner container found — never a context cancellation), the OpStartContainer
+// depending on it must not call ContainerStart. No ContainerStart expectation
+// is registered below: an unexpected call fails the test.
+func TestExecutePlanFailedPreStartGatesStart(t *testing.T) {
+	svc, apiClient, _ := newStartPhaseTestService(t)
+
+	service := types.ServiceConfig{
+		Name:     "web",
+		PreStart: []types.PreStartHook{{ContainerSpec: types.ContainerSpec{Command: types.ShellCommand{"init"}}}},
+	}
+	project := &types.Project{Name: "test", Services: types.Services{"web": service}}
+
+	apiClient.EXPECT().ContainerCreate(gomock.Any(), gomock.Any()).
+		Return(client.ContainerCreateResult{ID: "new-id"}, nil)
+	apiClient.EXPECT().ContainerInspect(gomock.Any(), "new-id", gomock.Any()).
+		Return(client.ContainerInspectResult{Container: container.InspectResponse{
+			ID:              "new-id",
+			Name:            "/test-web-1",
+			Config:          &container.Config{},
+			NetworkSettings: &container.NetworkSettings{},
+		}}, nil)
+	// Both the pre_start drift-check and the hook-runner scan list no
+	// container: no replica is running, and no runner was prepared for the
+	// declared hook — runPreStart fails with "no hook runner container
+	// found", a genuine error, not a cancellation.
+	apiClient.EXPECT().ContainerList(gomock.Any(), gomock.Any()).
+		Return(client.ContainerListResult{}, nil).Times(2)
+	// No ContainerStart expectation: registering one here would hide the
+	// bug by silently accepting the call this test exists to forbid.
+
+	plan := &Plan{}
+	create := plan.addNode(Operation{
+		Type:       OpCreateContainer,
+		ResourceID: "service:web:1",
+		Cause:      "no existing container",
+		Service:    &service,
+		Name:       "test-web-1",
+		Number:     1,
+	}, "")
+	preStart := plan.addNode(Operation{
+		Type:         OpRunPreStart,
+		ResourceID:   "service:web:1",
+		Cause:        "pre_start hooks",
+		Service:      &service,
+		CreateNodeID: create.ID,
+	}, "start:web:1", create)
+	plan.addNode(Operation{
+		Type:         OpStartContainer,
+		ResourceID:   "service:web:1",
+		Cause:        "start",
+		Service:      &service,
+		CreateNodeID: create.ID,
+	}, "start:web:1", preStart)
+
+	err := svc.executePlan(t.Context(), project, emptyObservedState("test"), plan)
+	assert.ErrorContains(t, err, "no hook runner container found")
+}