From 03708c61ce100bdfeb9fad459245865b5a655a50 Mon Sep 17 00:00:00 2001
From: Nicolas De Loof <nicolas.deloof@gmail.com>
Date: Tue, 27 Oct 2026 10:02:39 +0100
Subject: [PATCH 1/4] docs(compose): drop the comments that describe the
 deleted imperative start path

startService and startServiceContainer were removed by the migration of
compose start to the plan engine, yet many comments still cite them, or
isNotRunning, or describe the start as walked by InDependencyOrder.
Rewrite them to state the behavior itself, and fix the ones that say
which commands still walk the dependency order.

Comments only: the token stream of every Go file, comments excluded, is
unchanged.

Part of #14081

Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
---
 pkg/compose/executor.go           | 12 +++----
 pkg/compose/executor_events.go    |  5 ++-
 pkg/compose/executor_ops.go       |  8 ++---
 pkg/compose/observed_state.go     |  8 ++---
 pkg/compose/plan.go               |  6 ++--
 pkg/compose/reconcile.go          | 56 +++++++++++++++----------------
 pkg/compose/service_containers.go | 24 ++++++-------
 pkg/compose/start.go              | 26 +++++---------
 pkg/compose/up.go                 | 15 ++++-----
 9 files changed, 73 insertions(+), 87 deletions(-)

diff --git a/pkg/compose/executor.go b/pkg/compose/executor.go
index c10b25962d0..8ab02ae5320 100644
--- a/pkg/compose/executor.go
+++ b/pkg/compose/executor.go
@@ -39,8 +39,7 @@ type planExecutor struct {
 	project *types.Project
 	pctx    *reconciliationContext
 
-	// listener streams pre_start/post_start hook logs, exactly like the
-	// imperative start path. It is set once, when the Start phase is entered
+	// listener streams pre_start/post_start hook logs. It is set once, when the Start phase is entered
 	// (planRun.runStart): an attached caller only has one by then, its printer
 	// not existing before the Create phase is done. nil leaves hook execution
 	// silent, matching detached behavior.
@@ -379,8 +378,9 @@ func (p *phaseCancel) fail(node *PlanNode, events api.EventProcessor, groups *gr
 // latter needs a dependency that is already running, hence with no start
 // node for the wait to depend on, whose health relies on a sibling the plan
 // has yet to start: with a bounded --parallel the wait could take the slot
-// that sibling's start needs and only end at its timeout. The imperative
-// waitDependencies stays outside the cap for the same reason.
+// that sibling's start needs and only end at its timeout. The waits
+// waitDependencies runs outside any plan stay outside the cap for the same
+// reason.
 func acquireNodeSlot(ctx context.Context, limiter *semaphore.Weighted, node *PlanNode) (func(), error) {
 	if node.Operation.Type == OpWaitCondition {
 		return func() {}, nil
@@ -550,8 +550,8 @@ func (e *waitInterrupted) Error() string { return "wait interrupted" }
 
 // translateWaitTimeout turns a plan failure caused by a wait node's own
 // --wait-timeout window (see errWaitTimeout) into the "timeout waiting for
-// dependencies" message the imperative start path always reported for a
-// depends_on wait timing out, with or without --wait, and leaves any other
+// dependencies" message a depends_on wait timing out has always reported,
+// with or without --wait, and leaves any other
 // error as is. It is deliberately not the "application not healthy" message:
 // that one belongs to the final --wait readiness check that runs after the
 // plan, which has its own translation.
diff --git a/pkg/compose/executor_events.go b/pkg/compose/executor_events.go
index cc2bf4e7c16..c10a96e5cca 100644
--- a/pkg/compose/executor_events.go
+++ b/pkg/compose/executor_events.go
@@ -147,9 +147,8 @@ func (gt *groupTracker) onNodeStart(node *PlanNode, events api.EventProcessor) b
 // triggersWorking reports whether this node starting should fire the group's
 // Working event. A recreate group fires on its first node; a start group
 // fires specifically on OpStartContainer — pre_start hooks, when planned,
-// run silently before it, matching the imperative engine's startService,
-// which never surfaces a Starting event until the ContainerStart call itself
-// begins.
+// run silently before it: no Starting event is surfaced until the
+// ContainerStart call itself begins.
 func (gs *groupState) triggersWorking(t OperationType) bool {
 	if gs.kind == groupStart {
 		return t == OpStartContainer
diff --git a/pkg/compose/executor_ops.go b/pkg/compose/executor_ops.go
index 05e27622adb..d087489353e 100644
--- a/pkg/compose/executor_ops.go
+++ b/pkg/compose/executor_ops.go
@@ -133,7 +133,7 @@ func (exec *planExecutor) execCreateContainer(ctx context.Context, node *PlanNod
 // today only the create phase's exceptional-state restart of a paused/dead
 // container) is a plain ContainerStart. A start-phase operation (Service set)
 // performs the full service start — secret/config injection right before
-// ContainerStart — mirroring startServiceContainer; its target resolves
+// ContainerStart; its target resolves
 // either from the observed container or, for a replica the plan itself
 // creates, from the CreateContainer node's result (the same mechanism
 // OpRenameContainer already uses).
@@ -213,9 +213,9 @@ func (exec *planExecutor) resolveContainerSummary(op Operation) (container.Summa
 // execWaitCondition polls the dependency service named by the operation
 // until it satisfies the declared depends_on condition or the context ends —
 // the plan-side equivalent of one waitDependencies edge, reusing the exact
-// same polling primitive (waitDependency) and per-condition checks the
-// imperative engine uses, so events and error messages cannot drift between
-// the two. required: false dependencies are marked BestEffort by the
+// same polling primitive (waitDependency) and per-condition checks as
+// waitDependencies, so events and error messages cannot drift between the
+// two. required: false dependencies are marked BestEffort by the
 // reconciler: a missing dependency or a failed/timed-out condition is then a
 // warning, not a plan failure — matching waitDependencies' own
 // optional-dependency handling.
diff --git a/pkg/compose/observed_state.go b/pkg/compose/observed_state.go
index 0d4c088c0fe..026c932b4e6 100644
--- a/pkg/compose/observed_state.go
+++ b/pkg/compose/observed_state.go
@@ -215,8 +215,8 @@ func (s *composeService) collectObservedState(ctx context.Context, project *type
 	// phase's own RunPreStart never reads observed.HookContainers, it
 	// executes a runner the create phase already prepared or fails outright
 	// if none exists. Skipping this for ScopeStart matches the
-	// network/volume guard below: one more unconditional daemon call the old
-	// imperative engine never made, which could otherwise fail a no-op
+	// network/volume guard below: one more unconditional daemon call a start
+	// never made, which could otherwise fail a no-op
 	// `compose start` on a service with pre_start hooks declared (second
 	// Copilot review finding in the same vein as the first).
 	if scope.plansCreatePhase() {
@@ -265,8 +265,8 @@ func (s *composeService) collectObservedState(ctx context.Context, project *type
 	// reconcileNetworks/reconcileVolumes, which ScopeStart never runs, do).
 	// Skipping these calls for ScopeStart isn't just an optimization -- a
 	// Copilot review finding on #14296 noted that querying them unconditionally
-	// gives `compose start` new daemon dependencies the old imperative engine
-	// never had (only ever called getContainers): an authorization plugin
+	// gives `compose start` new daemon dependencies it never had (it only ever
+	// called getContainers): an authorization plugin
 	// permitting container list/start but denying network or volume listing
 	// would now make `compose start` fail before starting anything.
 	if scope.plansCreatePhase() {
diff --git a/pkg/compose/plan.go b/pkg/compose/plan.go
index 22efc8afe03..2a0e361b79c 100644
--- a/pkg/compose/plan.go
+++ b/pkg/compose/plan.go
@@ -134,14 +134,14 @@ type Operation struct {
 	Condition    string               // for OpWaitCondition: depends_on condition to wait for (service_healthy, ...)
 	HookIndex    int                  // for OpCreateHookContainer: position of the hook in the service's hook list
 	// RemoveVolumes asks OpRemoveContainer to also remove the container's
-	// anonymous volumes — the imperative semantics for hook-runner containers.
+	// anonymous volumes — what removing a hook-runner container has always done.
 	RemoveVolumes bool
 	// BestEffort marks an operation whose failure must not abort the plan.
 	// Used for the optional removal of the old network on a rename (if the
 	// network is still in use by non-Compose containers the removal is skipped
 	// with a warning — the new network already carries a different name), and
-	// for purging stale pre_start hook runners (the imperative purge is
-	// warn-only: a failed removal leaves the container visible, never blocks).
+	// for purging stale pre_start hook runners (a failed removal only warns:
+	// it leaves the container visible, never blocks).
 	BestEffort bool
 }
 
diff --git a/pkg/compose/reconcile.go b/pkg/compose/reconcile.go
index bfb6eb2c783..c09c2987039 100644
--- a/pkg/compose/reconcile.go
+++ b/pkg/compose/reconcile.go
@@ -122,8 +122,8 @@ type reconciler struct {
 	// r.project is never reassigned between the two calls.
 	dependencyGraph *Graph
 	// removedByPlan records replicas the create phase condemns (scale-down
-	// stop+remove): the start phase must never plan a start for them — the
-	// imperative engine only ever starts what survives the convergence.
+	// stop+remove): the start phase must never plan a start for them — only
+	// what survives the convergence is started.
 	removedByPlan map[string]bool
 	// hookRunnerNodes records, per service, the create-phase nodes that
 	// materialize the pre_start hook runners, so the start-phase RunPreStart
@@ -960,8 +960,8 @@ func (r *reconciler) reconcileService(service types.ServiceConfig) error {
 // per_replica return below. The start phase only executes runners prepared
 // here; it never creates one itself.
 //
-// Purges are best-effort (mirroring the historical imperative purge, which was
-// warn-only) but the creates depend on them: runner names are deterministic,
+// Purges are best-effort (a failed removal only warns) but the creates depend
+// on them: runner names are deterministic,
 // so a genuinely stuck old runner surfaces as a name conflict on the create.
 // The creates also depend on every replica node and on the infrastructure, so
 // the executor resolves the target replica (VolumesFrom) against a final live
@@ -1138,12 +1138,11 @@ func (r *reconciler) hasVolumeMismatch(expected types.ServiceConfig, oc Observed
 }
 
 // planStartPhase appends the Start phase to the plan: for every service, a
-// replica chain of start-phase operations reproducing the imperative
-// engine's semantics — dependency conditions first, one optional
-// OpRunPreStart when no replica was running at observation, then per
+// replica chain of start-phase operations — dependency conditions first, one
+// optional OpRunPreStart when no replica was running at observation, then per
 // replica inject+start (OpStartContainer, enriched by the executor) and
 // post_start hooks, each replica chained after the previous one to keep
-// today's sequential start order, now visible in the plan.
+// the sequential start order, visible in the plan.
 //
 // Conditions other than service_started materialize as OpWaitCondition
 // nodes, deduplicated per (service, condition) across dependents; health is
@@ -1257,12 +1256,12 @@ func (r *reconciler) exceptionalStateRestartNodes(service string) []*PlanNode {
 
 // startPhaseReplicas collects the replicas to start, ascending number:
 // containers materialized by the create phase plus observed up-to-date
-// containers not running — exactly the isNotRunning set the imperative start
-// phase acts on. anyRunning reports whether a replica will be running when
+// containers not running — exactly the set a start acts on. anyRunning
+// reports whether a replica will be running when
 // the start phase looks: running at observation and left untouched by the
 // plan, or brought to running by the create phase itself (the bare restart
-// of exceptional states). Both gate pre_start and take no start node — the
-// imperative engine neither re-starts nor injects into a running container.
+// of exceptional states). Both gate pre_start and take no start node — a
+// running container is neither re-started nor injected into.
 func (r *reconciler) startPhaseReplicas(service types.ServiceConfig) (replicas []startReplica, anyRunning bool) {
 	planned := r.containerNodes[service.Name]
 	seen := map[int]bool{}
@@ -1271,7 +1270,7 @@ func (r *reconciler) startPhaseReplicas(service types.ServiceConfig) (replicas [
 		resID := serviceReplicaID(service.Name, oc.Number)
 		if r.removedByPlan[resID] {
 			// condemned by the create phase (scale-down): gone by the time
-			// the imperative start phase would look, so it neither starts
+			// the start phase would look, so it neither starts
 			// nor counts as running for the pre_start gating
 			seen[oc.Number] = true
 			continue
@@ -1291,10 +1290,9 @@ func (r *reconciler) startPhaseReplicas(service types.ServiceConfig) (replicas [
 			if stopNode, stopped := r.stoppedByPlan[oc.ID]; stopped {
 				// the create phase stops this container WITHOUT recreating
 				// it (network recreate): it will be down when the start
-				// phase looks, and the imperative engine restarts it from
-				// its second snapshot — so does the plan, ordered after the
+				// phase looks, so the plan restarts it, ordered after the
 				// container's last reconnect (else its stop). Not running
-				// for the pre_start gating, like in the imperative engine.
+				// for the pre_start gating.
 				after := stopNode
 				if reconnects := r.connectNodes[oc.ID]; len(reconnects) > 0 {
 					after = reconnects[len(reconnects)-1]
@@ -1324,9 +1322,9 @@ func (r *reconciler) startPhaseReplicas(service types.ServiceConfig) (replicas [
 // prerequisites: a service_started condition is a plain edge to the
 // dependency's chain end, any other condition a deduplicated OpWaitCondition
 // node re-evaluated at execution time. Which dependencies need a wait node
-// is delegated to shouldWaitForDependency — the imperative engine's own
-// gate — errors included: a dependency that is neither declared nor
-// disabled is the same hard failure it is today.
+// is delegated to shouldWaitForDependency — the gate waitDependencies shares
+// — errors included: a dependency that is neither declared nor disabled is a
+// hard failure.
 func (r *reconciler) startPhaseDependencies(service types.ServiceConfig) ([]*PlanNode, error) {
 	var depNodes []*PlanNode
 	for _, dep := range sortedKeys(service.DependsOn) {
@@ -1386,8 +1384,8 @@ func (r *reconciler) waitConditionNode(dep string, cfg types.ServiceDependency,
 }
 
 // planProviderStart is the start-phase visit of a provider service: no
-// container to start, but its own depends_on still applies — the imperative
-// startService waits on it for every service, providers included. Its chain
+// container to start, but its own depends_on still applies, as for every
+// service, providers included. Its chain
 // end is its create-phase RunProvider node (the deployment itself), plus
 // those dependency prerequisites when it has any -- a consumer must wait for
 // both: the provider's own dependencies satisfied AND the provider actually
@@ -1431,8 +1429,8 @@ func (r *reconciler) planProviderStart(service types.ServiceConfig) error {
 
 // planProviderRelayRestart plans the start of the provider's own observed,
 // not-running containers when the create phase didn't already converge them
-// (see planProviderStart's caller). The imperative engine decided
-// bare-vs-enriched PER CONTAINER (isRelayContainer), never from the service's
+// (see planProviderStart's caller). Bare-vs-enriched is decided
+// PER CONTAINER (isRelayContainer), never from the service's
 // currently declared type: a provider: declaration can change without an
 // intervening `up`/create, so under pure ScopeStart the observed container
 // can be the real relay (bare start) or a stale non-relay replica left over
@@ -1467,7 +1465,7 @@ func (r *reconciler) planServiceStart(service types.ServiceConfig) error {
 	if service.Provider != nil {
 		return r.planProviderStart(service)
 	}
-	// imperative parity (startService): deploy.replicas: 0 is an unconditional
+	// deploy.replicas: 0 is an unconditional
 	// no-op, but a service with scale: 0 is still visited -- its depends_on
 	// conditions are evaluated and its dependents ordered after them -- and
 	// whatever replicas it already owns are started (scope Start only: the
@@ -1479,7 +1477,7 @@ func (r *reconciler) planServiceStart(service types.ServiceConfig) error {
 
 	replicas, anyRunning := r.startPhaseReplicas(service)
 	// dependency conditions are evaluated even when nothing has to start:
-	// the imperative engine calls waitDependencies for every visited service
+	// every visited service evaluates its dependencies
 	// BEFORE looking at what to start, so an up with everything running
 	// still fails on an unhealthy required dependency — and a dependency
 	// error wins over "no container to start", in that same order
@@ -1488,8 +1486,8 @@ func (r *reconciler) planServiceStart(service types.ServiceConfig) error {
 		return err
 	}
 	if len(replicas) == 0 && !anyRunning && r.options.Scope == ScopeStart && service.GetScale() > 0 && !r.observedAnyContainer() {
-		// imperative parity (startService): the "no container to start" check
-		// is project-wide -- it receives the whole project's container list,
+		// the "no container to start" check
+		// is project-wide -- it looks at the whole project's container list,
 		// so a service without a container next to existing ones is a no-op
 		// and only a project with no container at all fails. Only reachable
 		// under scope Start, since CreateStart would have planned the
@@ -1519,7 +1517,7 @@ func (r *reconciler) planServiceStart(service types.ServiceConfig) error {
 		// no entry otherwise (ScopeStart, all running, no depends_on): the
 		// visit had no effect, so dependents have nothing to order after —
 		// a service_started condition on a running dependency is already
-		// satisfied, exactly like the imperative engine's empty visit
+		// satisfied, as after an empty visit
 		return nil
 	}
 
@@ -1538,7 +1536,7 @@ func (r *reconciler) chainStartReplicas(service types.ServiceConfig, replicas []
 	}
 
 	// pre_start runs once per service, only when no replica was running at
-	// observation — the imperative gating (initial up, force-recreate, or
+	// observation (initial up, force-recreate, or
 	// spec change), decided at plan time. Never against a stale relay left
 	// over from before the service's provider: declaration was removed
 	// (isReplicaRelay) -- it has no shell/process for the hook's VolumesFrom
diff --git a/pkg/compose/service_containers.go b/pkg/compose/service_containers.go
index 3010658ff0e..185a1008d2c 100644
--- a/pkg/compose/service_containers.go
+++ b/pkg/compose/service_containers.go
@@ -38,12 +38,13 @@ import (
 	"github.com/docker/compose/v5/pkg/api"
 )
 
-// This file gathers the per-service container helpers shared by both
-// lifecycle engines: the plan-based reconciler (reconcile.go, entered through
-// create/up) and the imperative dependency-ordered engine (dependencies.go,
-// used by start/stop/restart/down). It covers container naming, resolution of
-// service references (volumes_from, network_mode/ipc/pid, links), dependency
-// waiting, container creation through the Docker API, and service startup.
+// This file gathers the per-service container helpers shared by the
+// plan-based engine (reconcile.go and the executor, entered through
+// create/up/start) and by the commands that still walk the dependency order
+// themselves (dependencies.go: restart, stop, down and the classic build). It
+// covers container naming, resolution of service references (volumes_from,
+// network_mode/ipc/pid, links), dependency waiting and container creation
+// through the Docker API.
 
 const (
 	doubledContainerNameWarning = "WARNING: The %q service is using the custom container name %q. " +
@@ -266,7 +267,7 @@ func (s *composeService) waitDependencyReached(ctx, origCtx context.Context, dep
 			// Every condition this switch doesn't handle explicitly ends up
 			// here: ServiceConditionStarted is filtered out before this
 			// function is ever called (shouldWaitForDependency — "already
-			// managed by InDependencyOrder"), so nothing reaching this
+			// ensured by the dependency order"), so nothing reaching this
 			// branch is ever a value this runtime actually understands.
 			// That covers a compose file with an unrecognized condition
 			// (rejected by schema.Validate before it can even get this far
@@ -371,16 +372,15 @@ func (s *composeService) checkDependencyCompleted(ctx context.Context, dep strin
 	return false, errors.New(msg)
 }
 
-// errNoContainerToStart is shared by the imperative engine (startService)
-// and the plan reconciler (planServiceStart) so the message cannot drift —
-// the plan's fidelity contract is word for word.
+// errNoContainerToStart is the failure of a service the start phase finds no
+// container to start for (see planServiceStart).
 func errNoContainerToStart(service string) error {
 	return fmt.Errorf("service %q has no container to start", service)
 }
 
 func shouldWaitForDependency(serviceName string, dependencyConfig types.ServiceDependency, project *types.Project) (bool, error) {
 	if dependencyConfig.Condition == types.ServiceConditionStarted {
-		// already managed by InDependencyOrder
+		// already ensured by the dependency order
 		return false, nil
 	}
 	if service, err := project.GetService(serviceName); err != nil {
@@ -444,7 +444,7 @@ func (s *composeService) createContainer(ctx context.Context, project *types.Pro
 // engine allocates published ports from ranges non-atomically, and two
 // concurrent starts can be assigned the same port. Every code path calling
 // ContainerStart on a service container must hold it (the plan executor's
-// execStartContainer and the imperative startServiceContainer both do).
+// execStartContainer does, on both of its paths).
 //
 // It deliberately stays a package-level variable rather than a composeService
 // field: the race lives in the daemon, not in a service instance, so two
diff --git a/pkg/compose/start.go b/pkg/compose/start.go
index b4541694758..fba04369a7c 100644
--- a/pkg/compose/start.go
+++ b/pkg/compose/start.go
@@ -33,13 +33,10 @@ func (s *composeService) Start(ctx context.Context, projectName string, options
 	}, "start", s.events)
 }
 
-// start builds and executes a Start-only plan (epic #14081, lot 2): the
-// semantic switchover from InDependencyOrder's imperative traversal
-// (startService/waitDependencies) to the same reconciler/executor pair every
-// other lifecycle command already uses. ScopeStart's planStartPhase works
-// straight off the observed containers (see TestPlanStart_StartOnlyScope) --
-// it never converges anything, matching start()'s own historical contract of
-// starting what exists, never creating. The Start phase of an interactive up
+// start builds and executes a Start-only plan. ScopeStart's planStartPhase
+// works straight off the observed containers (see
+// TestPlanStart_StartOnlyScope) -- it never converges anything: start()
+// starts what exists, never creating. The Start phase of an interactive up
 // does not go through here: it runs from the up's own plan, and hands its
 // session's listener to runStart (see runInteractiveUp).
 func (s *composeService) start(ctx context.Context, projectName string, options api.StartOptions) error {
@@ -83,19 +80,14 @@ func (s *composeService) start(ctx context.Context, projectName string, options
 
 	// Unlike upDetached/create, start() never calls emitRunningEvents: a
 	// no-op `compose start` (every container already running) must stay
-	// silent, matching the imperative engine's own contract (see the now
-	// deleted TestStartService_AlreadyRunningIsSilent) -- a Copilot review
-	// finding on the first version of this migration, which had copied
-	// upDetached's call without checking start()'s narrower UX contract.
-	// emitRunningEvents is create-oriented: it exists so `up`/`create`'s full
+	// silent. emitRunningEvents is create-oriented: it exists so `up`/`create`'s full
 	// project-status display accounts for containers the plan won't touch,
 	// a concern start() has never had.
 
-	// start()'s own dependency waits used to thread WaitTimeout through every
-	// wait unconditionally (InDependencyOrder → startService, regardless of
-	// options.Wait -- --wait-timeout alone, with no --wait, is a legal CLI
-	// combination nothing rejects). exec.waitTimeout reproduces that for the
-	// plan's own OpWaitCondition nodes, each with its own fresh window
+	// WaitTimeout bounds every dependency wait of start() unconditionally,
+	// regardless of options.Wait (--wait-timeout alone, with no --wait, is a
+	// legal CLI combination nothing rejects). exec.waitTimeout applies it to
+	// the plan's own OpWaitCondition nodes, each with its own fresh window
 	// starting when that wait begins -- see upDetached's identical reasoning.
 	exec := s.newPlanExecutor(project, observed)
 	exec.waitTimeout = options.WaitTimeout
diff --git a/pkg/compose/up.go b/pkg/compose/up.go
index 53695f9c3c3..dc07f85548e 100644
--- a/pkg/compose/up.go
+++ b/pkg/compose/up.go
@@ -113,18 +113,15 @@ func (s *composeService) prepareUp(ctx context.Context, project *types.Project,
 
 	// The plan's own OpWaitCondition nodes (service_healthy /
 	// service_completed_successfully dependency waits) block with no timeout
-	// of their own -- unlike start()'s per-dependency waitDependencies call,
-	// which already threads WaitTimeout through every wait unconditionally
-	// (InDependencyOrder passes it to startService regardless of
-	// options.Wait -- --wait-timeout alone, with no --wait, is a legal CLI
-	// combination nothing rejects). exec.waitTimeout bounds exactly those
-	// nodes (see executeNode), each with its OWN fresh window starting when
-	// that wait begins -- not a single shared budget counted down from
+	// of their own: WaitTimeout bounds every dependency wait unconditionally,
+	// regardless of options.Wait (--wait-timeout alone, with no --wait, is a
+	// legal CLI combination nothing rejects). exec.waitTimeout bounds exactly
+	// those nodes (see executeNode), each with its OWN fresh window starting
+	// when that wait begins -- not a single shared budget counted down from
 	// before the plan even started. Sharing one absolute deadline across
 	// create/start/hook work and every wait would let slow, unrelated work
 	// alone exhaust it, failing an otherwise healthy deployment as "not
-	// healthy" with no wait ever actually timing out -- start()'s real,
-	// per-wait-independent behavior never had that failure mode.
+	// healthy" with no wait ever actually timing out.
 	exec := s.newPlanExecutor(project, observed)
 	exec.waitTimeout = options.Start.WaitTimeout
 	return project, exec, plan, nil

From 1609b3b8d9ce738e51595228c6cb30d1cbe3343a Mon Sep 17 00:00:00 2001
From: Nicolas De Loof <nicolas.deloof@gmail.com>
Date: Tue, 27 Oct 2026 10:03:19 +0100
Subject: [PATCH 2/4] docs(compose): drop the stale imperative-engine
 references of the tests

Same cleanup as the previous commit, for the comments of the unit tests:
they cite startService, isNotRunning and the "old imperative engine" as
the source of a behavior the plan now owns.

Comments only: the token stream of every Go file, comments excluded, is
unchanged.

Part of #14081

Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
---
 pkg/compose/executor_start_test.go  |  9 ++----
 pkg/compose/executor_test.go        |  2 +-
 pkg/compose/observed_state_test.go  |  4 +--
 pkg/compose/reconcile_start_test.go | 46 ++++++++++++++---------------
 pkg/compose/reconcile_test.go       |  5 ++--
 5 files changed, 31 insertions(+), 35 deletions(-)

diff --git a/pkg/compose/executor_start_test.go b/pkg/compose/executor_start_test.go
index ba951c5d1f5..2bfc1448f5b 100644
--- a/pkg/compose/executor_start_test.go
+++ b/pkg/compose/executor_start_test.go
@@ -114,7 +114,7 @@ func TestExecWaitCondition_OptionalMissingDependencyIsTolerated(t *testing.T) {
 // TestExecWaitCondition_HealthyConditionSatisfied drives execWaitCondition
 // against a live dependency container that reports healthy on the first
 // probe, reusing the exact same isServiceHealthy/waitDependency primitives
-// the imperative engine's waitDependencies uses.
+// waitDependencies uses.
 func TestExecWaitCondition_HealthyConditionSatisfied(t *testing.T) {
 	svc, apiClient, recorder := newStartPhaseTestService(t)
 
@@ -154,9 +154,7 @@ func TestExecWaitCondition_HealthyConditionSatisfied(t *testing.T) {
 
 // TestExecutePlanWaitConditionRespectsContextDeadline is a regression test
 // for a gap a reviewer caught on upDetached (pkg/compose/up.go): the plan's
-// own OpWaitCondition nodes have no timeout of their own (unlike the
-// imperative engine's waitDependencies, which already threaded WaitTimeout
-// through every per-dependency wait, not just the final --wait check) --
+// own OpWaitCondition nodes have no timeout of their own --
 // they rely entirely on whatever ctx they're given (directly, or via
 // exec.waitTimeout -- see TestExecutePlanWaitDeadlineOnlyBoundsWaitNodes for
 // that path specifically). This drives a real OpWaitCondition node
@@ -1020,8 +1018,7 @@ func TestExecutePlanStartPhaseEventContract(t *testing.T) {
 // container.Summary — CreateNodeID only), exercising resolveContainerSummary's
 // live-view lookup. It asserts the group emits exactly one Starting→Started
 // pair, with Starting firing at the StartContainer node — not at
-// RunPreStart, which must run silently first, matching the imperative
-// engine's startService.
+// RunPreStart, which must run silently first.
 func TestExecutePlanStartPhaseFullChainThroughCreateNode(t *testing.T) {
 	svc, apiClient, recorder := newStartPhaseTestService(t)
 
diff --git a/pkg/compose/executor_test.go b/pkg/compose/executor_test.go
index e8da0c64f5c..dcde72f8a41 100644
--- a/pkg/compose/executor_test.go
+++ b/pkg/compose/executor_test.go
@@ -227,7 +227,7 @@ func emptyObservedState(project string) *ObservedState {
 // wiring and group tracker — not a hand-rolled loop over executeNode.
 // A best-effort removal failure (stale pre_start hook runner purge) is
 // warn-only: the plan carries on, and the removal passes RemoveVolumes so the
-// runner's anonymous volumes go with it — the imperative purge semantics.
+// runner's anonymous volumes go with it.
 func TestExecutePlanBestEffortRemoveContainerFailureTolerated(t *testing.T) {
 	svc, apiClient := newTestService(t)
 
diff --git a/pkg/compose/observed_state_test.go b/pkg/compose/observed_state_test.go
index 7b05ae36d95..8926569585c 100644
--- a/pkg/compose/observed_state_test.go
+++ b/pkg/compose/observed_state_test.go
@@ -273,8 +273,8 @@ func TestCollectObservedState(t *testing.T) {
 // test for a Copilot review finding on #14296: ScopeStart's planStartPhase
 // never reads resolvedNetworks/resolvedVolumes (only reconcileNetworks/
 // reconcileVolumes, which ScopeStart never runs, do), so querying them
-// unconditionally gave `compose start` new daemon dependencies the old
-// imperative engine never had (only ever called getContainers) -- a problem
+// unconditionally gave `compose start` new daemon dependencies it never had
+// (it only ever called getContainers) -- a problem
 // for an authorization plugin permitting container list/start but denying
 // network or volume listing. No NetworkList/VolumeList/NetworkInspect/
 // VolumeInspect expectation is registered here: gomock fails the test if
diff --git a/pkg/compose/reconcile_start_test.go b/pkg/compose/reconcile_start_test.go
index 1cdc2da73f5..f6dc48fd784 100644
--- a/pkg/compose/reconcile_start_test.go
+++ b/pkg/compose/reconcile_start_test.go
@@ -166,7 +166,7 @@ func TestPlanStart_HooksAndReplicaChain(t *testing.T) {
 
 // With a replica already running and untouched by the plan, pre_start is
 // gated off and the running replica gets no node — only the non-running one
-// starts, the imperative isNotRunning role expressed in the plan.
+// starts.
 func TestPlanStart_RunningReplicaGatesPreStart(t *testing.T) {
 	two := 2
 	service := types.ServiceConfig{
@@ -232,9 +232,9 @@ func TestPlanStart_RecreatedReplicaTargetsCreateNode(t *testing.T) {
 }
 
 // An exceptional-state container (paused, dead, ...) gets its bare
-// create-phase restart and NOTHING in the start phase: the imperative engine
-// finds it running when the start phase looks, so it neither re-starts nor
-// injects — and its being running gates pre_start for the whole service.
+// create-phase restart and NOTHING in the start phase: it is running when the
+// start phase looks, so it neither re-starts nor injects — and its being
+// running gates pre_start for the whole service.
 func TestPlanStart_ExceptionalStateReplicaCountsAsRunning(t *testing.T) {
 	hook := types.PreStartHook{}
 	hook.Command = []string{"echo", "hi"}
@@ -268,9 +268,9 @@ func TestPlanStart_ExceptionalStateReplicaCountsAsRunning(t *testing.T) {
 	assert.Assert(t, createPhase != nil, "the historical bare restart must remain:\n%s", plan)
 }
 
-// Dependency conditions are re-verified even when nothing has to start — the
-// imperative engine calls waitDependencies for every visited service before
-// looking at what to start, so an up with everything running still fails on
+// Dependency conditions are re-verified even when nothing has to start — every
+// visited service evaluates its dependencies before looking at what to start,
+// so an up with everything running still fails on
 // an unhealthy required dependency. The dependent's visit end (the wait) also
 // orders whoever depends on IT via service_started.
 func TestPlanStart_AllRunningStillWaitsOnConditions(t *testing.T) {
@@ -307,7 +307,7 @@ func TestPlanStart_AllRunningStillWaitsOnConditions(t *testing.T) {
 	assert.Assert(t, !wait.Operation.BestEffort)
 }
 
-// Imperative parity (startService): a service with scale: 0 is still visited
+// A service with scale: 0 is still visited
 // -- its depends_on conditions are evaluated before the no-op, so an unhealthy
 // dependency still fails `up`/`start` and whatever is ordered after the
 // service is ordered after those prerequisites -- and under scope Start the
@@ -370,7 +370,7 @@ func TestPlanStart_ScaleZeroIsStillVisited(t *testing.T) {
 	}
 }
 
-// Imperative parity (startService): under scope Start, a scale>0 service
+// Under scope Start, a scale>0 service
 // with no container at all fails the plan the way `compose start` fails.
 func TestPlanStart_NoContainerToStartFails(t *testing.T) {
 	project := &types.Project{
@@ -382,7 +382,7 @@ func TestPlanStart_NoContainerToStartFails(t *testing.T) {
 	assert.ErrorContains(t, err, `service "app" has no container to start`)
 }
 
-// Imperative parity (startService): the "no container to start" check is
+// The "no container to start" check is
 // project-wide. A service without a container next to existing ones is a
 // no-op for `compose start` (the other services still start); only a project
 // with no container at all fails.
@@ -467,7 +467,7 @@ func TestPlanStart_OrdinaryPathSkipsPreStartWhenLowestIsRelay(t *testing.T) {
 }
 
 // A depends_on naming a service that is neither declared nor disabled is
-// the same hard error the imperative engine raises (shouldWaitForDependency
+// a hard error (shouldWaitForDependency
 // propagates it) — reachable via `compose start` without a compose file,
 // where projectFromName never populates DisabledServices.
 func TestPlanStart_MissingDependencyFails(t *testing.T) {
@@ -487,13 +487,13 @@ func TestPlanStart_MissingDependencyFails(t *testing.T) {
 	assert.ErrorContains(t, err, "ghost")
 
 	// ordering parity: with NO container at all under scope Start, the
-	// dependency error still wins over errNoContainerToStart — the
-	// imperative startService waits on dependencies first
+	// dependency error still wins over errNoContainerToStart — dependencies
+	// are evaluated first
 	_, err = reconcile(t.Context(), project, emptyObserved(), startScopeOptions(ScopeStart), noPrompt)
 	assert.ErrorContains(t, err, "ghost")
 }
 
-// Imperative parity (startService): waitDependencies runs for every service,
+// Dependencies are evaluated for every service,
 // providers included — a provider's own depends_on must produce its wait
 // nodes instead of being skipped with the container-start machinery.
 func TestPlanStart_ProviderWaitsOnDependencies(t *testing.T) {
@@ -692,7 +692,7 @@ func TestPlanStart_ScaledExceptionalStateConsumerWaitsForEveryRestart(t *testing
 }
 
 // A replica condemned by scale-down must never receive a start-phase node:
-// the imperative engine only starts what survives the convergence — and the
+// only what survives the convergence is started — and the
 // condemned replica does not count as running for the pre_start gating.
 func TestPlanStart_ScaleDownTargetIsNeverStarted(t *testing.T) {
 	one := 1
@@ -730,7 +730,7 @@ func TestPlanStart_ScaleDownTargetIsNeverStarted(t *testing.T) {
 // A running container the create phase stops WITHOUT recreating (network
 // recreate) is down when the start phase looks: the plan restarts it —
 // ordered after its last reconnect — and it does not gate pre_start, exactly
-// like the imperative engine restarting it from its second snapshot.
+// as the container is down when the start phase looks.
 func TestStartPhaseReplicas_StoppedByPlanRestarts(t *testing.T) {
 	stop := &PlanNode{ID: 1, Operation: Operation{Type: OpStopContainer}}
 	reconnect := &PlanNode{ID: 2, Operation: Operation{Type: OpConnectNetwork}}
@@ -992,9 +992,9 @@ func TestPlanStart_OrdinaryPathBareStartsAStaleRelay(t *testing.T) {
 // test for the second Copilot review finding on #14296, the symmetric gap:
 // a service declared provider: whose observed container is NOT a relay (a
 // stale normal replica left over from before the service became
-// provider-backed) must still be started -- the OLD imperative engine's
-// startService only special-cased Provider != nil for the ZERO-containers
-// case, so an existing non-relay container was always started normally,
+// provider-backed) must still be started -- Provider != nil is only
+// special-cased for the ZERO-containers
+// case, so an existing non-relay container is always started normally,
 // including post_start hooks. planProviderRelayRestart's first version
 // silently dropped this case (filtered on isRelayContainer), returning
 // success without starting anything.
@@ -1033,8 +1033,8 @@ func TestPlanStart_ProviderPathEnrichesStaleNonRelayContainer(t *testing.T) {
 	assert.Assert(t, start.Operation.Service != nil,
 		"a stale non-relay container must get an enriched start (injection applies), like any ordinary replica:\n%s", plan)
 	// A Copilot review finding on the post_start-only version of this fix:
-	// pre_start must also run for this path, exactly like the imperative
-	// engine's own lowestNumberedContainer(toStart) + isRelayContainer gate.
+	// pre_start must also run for this path, gated on the lowest-numbered
+	// container to start and on isRelayContainer.
 	if pre == nil {
 		t.Fatalf("expected pre_start hooks to run before starting prov's stale non-relay container:\n%s", plan)
 	}
@@ -1094,8 +1094,8 @@ func TestPlanStart_ProviderPathChainsMultipleStaleContainers(t *testing.T) {
 // replica a relay?" on every iteration of the chain-building loop, so when
 // the lowest-numbered stale container was the relay (skipped) but a later
 // one was an ordinary stale replica, pre_start still ran against that later
-// one. The old imperative engine's own gate -- lowestNumberedContainer(toStart)
-// checked once -- never falls through like that: if its single candidate is
+// one. The gate -- the lowest-numbered container to start, checked once --
+// never falls through like that: if its single candidate is
 // a relay, pre_start is skipped entirely for the whole chain, with no
 // fallback to a later non-relay container.
 func TestPlanStart_ProviderPathPreStartGateDoesNotRetryPastARelay(t *testing.T) {
diff --git a/pkg/compose/reconcile_test.go b/pkg/compose/reconcile_test.go
index 5e2e00f4062..b70184cfacb 100644
--- a/pkg/compose/reconcile_test.go
+++ b/pkg/compose/reconcile_test.go
@@ -1336,9 +1336,8 @@ func TestReconcileContainers_ExitedIsNoop(t *testing.T) {
 // depends_on could start before their dependencies' operations complete.
 // Stale pre_start hook runners (left by a previous run that failed before
 // removing them) are purged by the plan when pre_start is going to run again:
-// hooks declared and no replica running at observation — the imperative
-// gating. Removals are best-effort and drop the runner's anonymous volumes,
-// like the warn-only imperative purge they mirror.
+// hooks declared and no replica running at observation. Removals are
+// best-effort (warn-only) and drop the runner's anonymous volumes.
 func TestReconcileContainers_StaleHookRunnersPurged(t *testing.T) {
 	project := &types.Project{
 		Name: "myproject",

From 21daf653aac4fba6df49e0ef3a91ff35b1db3086 Mon Sep 17 00:00:00 2001
From: Nicolas De Loof <nicolas.deloof@gmail.com>
Date: Tue, 27 Oct 2026 10:05:41 +0100
Subject: [PATCH 3/4] docs(compose): rewrap the rewritten comments and tighten
 four of them

Follow-up of the two previous commits, from their review: rewrap the
comments they left with an orphan line, give two of them back a subject,
point one at the plan's wait nodes, and drop the "imperative engine"
wording of the restart tests, restart being the one command that still
walks the dependency order itself.

Comments only: the token stream of every Go file, comments excluded, is
unchanged.

Part of #14081

Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
---
 pkg/compose/executor.go             | 12 ++++++------
 pkg/compose/reconcile.go            | 24 ++++++++++++------------
 pkg/compose/reconcile_start_test.go |  6 +++---
 pkg/compose/restart_test.go         | 10 +++++-----
 pkg/compose/start.go                |  3 ++-
 5 files changed, 28 insertions(+), 27 deletions(-)

diff --git a/pkg/compose/executor.go b/pkg/compose/executor.go
index 8ab02ae5320..182d8456c79 100644
--- a/pkg/compose/executor.go
+++ b/pkg/compose/executor.go
@@ -39,10 +39,10 @@ type planExecutor struct {
 	project *types.Project
 	pctx    *reconciliationContext
 
-	// listener streams pre_start/post_start hook logs. It is set once, when the Start phase is entered
-	// (planRun.runStart): an attached caller only has one by then, its printer
-	// not existing before the Create phase is done. nil leaves hook execution
-	// silent, matching detached behavior.
+	// listener streams pre_start/post_start hook logs. It is set once, when
+	// the Start phase is entered (planRun.runStart): an attached caller only
+	// has one by then, its printer not existing before the Create phase is
+	// done. nil leaves hook execution silent, matching detached behavior.
 	listener api.ContainerEventListener
 
 	// containersByService is a live view used to resolve service references
@@ -551,8 +551,8 @@ func (e *waitInterrupted) Error() string { return "wait interrupted" }
 // translateWaitTimeout turns a plan failure caused by a wait node's own
 // --wait-timeout window (see errWaitTimeout) into the "timeout waiting for
 // dependencies" message a depends_on wait timing out has always reported,
-// with or without --wait, and leaves any other
-// error as is. It is deliberately not the "application not healthy" message:
+// with or without --wait, and leaves any other error as is. It is
+// deliberately not the "application not healthy" message:
 // that one belongs to the final --wait readiness check that runs after the
 // plan, which has its own translation.
 func translateWaitTimeout(err error) error {
diff --git a/pkg/compose/reconcile.go b/pkg/compose/reconcile.go
index c09c2987039..fe1886b06a5 100644
--- a/pkg/compose/reconcile.go
+++ b/pkg/compose/reconcile.go
@@ -961,8 +961,8 @@ func (r *reconciler) reconcileService(service types.ServiceConfig) error {
 // here; it never creates one itself.
 //
 // Purges are best-effort (a failed removal only warns) but the creates depend
-// on them: runner names are deterministic,
-// so a genuinely stuck old runner surfaces as a name conflict on the create.
+// on them: runner names are deterministic, so a genuinely stuck old runner
+// surfaces as a name conflict on the create.
 // The creates also depend on every replica node and on the infrastructure, so
 // the executor resolves the target replica (VolumesFrom) against a final live
 // view with networks in place. When a running replica survives the plan, the
@@ -1385,8 +1385,8 @@ func (r *reconciler) waitConditionNode(dep string, cfg types.ServiceDependency,
 
 // planProviderStart is the start-phase visit of a provider service: no
 // container to start, but its own depends_on still applies, as for every
-// service, providers included. Its chain
-// end is its create-phase RunProvider node (the deployment itself), plus
+// service, providers included. Its chain end is its create-phase
+// RunProvider node (the deployment itself), plus
 // those dependency prerequisites when it has any -- a consumer must wait for
 // both: the provider's own dependencies satisfied AND the provider actually
 // deployed, not either in isolation. Before this ordering was corrected, a
@@ -1465,8 +1465,8 @@ func (r *reconciler) planServiceStart(service types.ServiceConfig) error {
 	if service.Provider != nil {
 		return r.planProviderStart(service)
 	}
-	// deploy.replicas: 0 is an unconditional
-	// no-op, but a service with scale: 0 is still visited -- its depends_on
+	// deploy.replicas: 0 is an unconditional no-op, but a service with
+	// scale: 0 is still visited -- its depends_on
 	// conditions are evaluated and its dependents ordered after them -- and
 	// whatever replicas it already owns are started (scope Start only: the
 	// create phase removes them all under CreateStart). It is just never an
@@ -1477,17 +1477,17 @@ func (r *reconciler) planServiceStart(service types.ServiceConfig) error {
 
 	replicas, anyRunning := r.startPhaseReplicas(service)
 	// dependency conditions are evaluated even when nothing has to start:
-	// every visited service evaluates its dependencies
-	// BEFORE looking at what to start, so an up with everything running
-	// still fails on an unhealthy required dependency — and a dependency
+	// every visited service evaluates its dependencies (the plan's wait
+	// nodes) BEFORE looking at what to start, so an up with everything
+	// running still fails on an unhealthy required dependency — and a dependency
 	// error wins over "no container to start", in that same order
 	depNodes, err := r.startPhaseDependencies(service)
 	if err != nil {
 		return err
 	}
 	if len(replicas) == 0 && !anyRunning && r.options.Scope == ScopeStart && service.GetScale() > 0 && !r.observedAnyContainer() {
-		// the "no container to start" check
-		// is project-wide -- it looks at the whole project's container list,
+		// the "no container to start" check is project-wide -- it looks at
+		// the whole project's container list,
 		// so a service without a container next to existing ones is a no-op
 		// and only a project with no container at all fails. Only reachable
 		// under scope Start, since CreateStart would have planned the
@@ -1517,7 +1517,7 @@ func (r *reconciler) planServiceStart(service types.ServiceConfig) error {
 		// no entry otherwise (ScopeStart, all running, no depends_on): the
 		// visit had no effect, so dependents have nothing to order after —
 		// a service_started condition on a running dependency is already
-		// satisfied, as after an empty visit
+		// satisfied, as when nothing needs starting
 		return nil
 	}
 
diff --git a/pkg/compose/reconcile_start_test.go b/pkg/compose/reconcile_start_test.go
index f6dc48fd784..501065aed51 100644
--- a/pkg/compose/reconcile_start_test.go
+++ b/pkg/compose/reconcile_start_test.go
@@ -992,9 +992,9 @@ func TestPlanStart_OrdinaryPathBareStartsAStaleRelay(t *testing.T) {
 // test for the second Copilot review finding on #14296, the symmetric gap:
 // a service declared provider: whose observed container is NOT a relay (a
 // stale normal replica left over from before the service became
-// provider-backed) must still be started -- Provider != nil is only
-// special-cased for the ZERO-containers
-// case, so an existing non-relay container is always started normally,
+// provider-backed) must still be started -- the start path only
+// special-cases Provider != nil for the ZERO-containers case, so an
+// existing non-relay container is always started normally,
 // including post_start hooks. planProviderRelayRestart's first version
 // silently dropped this case (filtered on isRelayContainer), returning
 // success without starting anything.
diff --git a/pkg/compose/restart_test.go b/pkg/compose/restart_test.go
index 101d8df22b3..f74dd8f6ed6 100644
--- a/pkg/compose/restart_test.go
+++ b/pkg/compose/restart_test.go
@@ -33,11 +33,11 @@ import (
 	"github.com/docker/compose/v5/pkg/api"
 )
 
-// These tests characterize the restart path before the lifecycle engines
-// converge (#14081): which depends_on relations survive project preparation,
-// and the per-container sequence (pre_stop → ContainerRestart → post_start)
-// with its events. restart stays on the imperative engine but shares the
-// hook/wait primitives with the plan.
+// These tests characterize the restart path: which depends_on relations
+// survive project preparation, and the per-container sequence (pre_stop →
+// ContainerRestart → post_start) with its events. restart still walks the
+// dependency order itself (InDependencyOrder) but shares the hook/wait
+// primitives with the plan.
 
 // TestPrepareRestartProject locks the selection semantics: only depends_on
 // relations declaring restart:true are kept, and naming services includes
diff --git a/pkg/compose/start.go b/pkg/compose/start.go
index fba04369a7c..e67f46493f5 100644
--- a/pkg/compose/start.go
+++ b/pkg/compose/start.go
@@ -80,7 +80,8 @@ func (s *composeService) start(ctx context.Context, projectName string, options
 
 	// Unlike upDetached/create, start() never calls emitRunningEvents: a
 	// no-op `compose start` (every container already running) must stay
-	// silent. emitRunningEvents is create-oriented: it exists so `up`/`create`'s full
+	// silent. emitRunningEvents is create-oriented: it exists so
+	// `up`/`create`'s full
 	// project-status display accounts for containers the plan won't touch,
 	// a concern start() has never had.
 

From 8e9a9a92bd1b18acba3a220e9d0504e5eaa328d7 Mon Sep 17 00:00:00 2001
From: Nicolas De Loof <nicolas.deloof@gmail.com>
Date: Tue, 27 Oct 2026 11:04:59 +0100
Subject: [PATCH 4/4] docs(compose): tie startMx to docker/compose#12846 and
 mark it for removal

startMx works around duplicate published host ports on Engine 28.0 to
28.2 (docker/compose#12846), a daemon port-allocator flaw fixed in Engine
28.3.0 (moby/moby#50054). Say so in its comment, state which start paths
take it, and add a TODO to remove it once Engine 28.x is out of support.

Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
---
 pkg/compose/service_containers.go | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/pkg/compose/service_containers.go b/pkg/compose/service_containers.go
index 185a1008d2c..3a9a84dc1b1 100644
--- a/pkg/compose/service_containers.go
+++ b/pkg/compose/service_containers.go
@@ -440,17 +440,23 @@ func (s *composeService) createContainer(ctx context.Context, project *types.Pro
 	return ctr, nil
 }
 
-// startMx serializes ContainerStart calls across the whole process: the
-// engine allocates published ports from ranges non-atomically, and two
-// concurrent starts can be assigned the same port. Every code path calling
-// ContainerStart on a service container must hold it (the plan executor's
-// execStartContainer does, on both of its paths).
+// startMx serializes the plan executor's ContainerStart calls across the whole
+// process. It is a workaround for docker/compose#12846 (added by #12851): with
+// Engine 28.0 to 28.2, containers started concurrently could be assigned the
+// same published host port, a flaw of the daemon's port allocator. That flaw
+// is fixed since Engine 28.3.0 (moby/moby#50054), so beyond it the mutex serves
+// no purpose, and it is unclear it ever prevented the duplicates reported.
+// Only this path takes it: restart, run, the relay and the hook runners start
+// their containers without it.
 //
-// It deliberately stays a package-level variable rather than a composeService
-// field: the race lives in the daemon, not in a service instance, so two
-// instances (e.g. SDK users) driving the same daemon must still exclude each
-// other. A per-instance lock would silently reintroduce the duplicate-port
-// assignment between them.
+// TODO: remove startMx, and the locking around it, once Engine 28.x is out of
+// support.
+//
+// While it exists, it deliberately stays a package-level variable rather than
+// a composeService field: the race lives in the daemon, not in a service
+// instance, so two instances (e.g. SDK users) driving the same daemon must
+// still exclude each other. A per-instance lock would silently reintroduce the
+// duplicate-port assignment between them.
 var startMx sync.Mutex
 
 func (s *composeService) createMobyContainer(ctx context.Context, project *types.Project, service types.ServiceConfig,