From 77ec74ee6718f5e5c77c3ce7bd4ab89cb57d57e5 Mon Sep 17 00:00:00 2001 From: Guillaume Lours Date: Thu, 28 May 2026 17:07:42 +0200 Subject: [PATCH] reconcile: document the asymmetric removeDeps in planRecreateContainer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit External reviewer noted that the alreadyStopped branch adds createNode to removeDeps while the !alreadyStopped branch does not — semantically correct but fragile, since it relies on the implicit invariant that stopNode.DependsOn contains createNode in the !alreadyStopped path. Spell out the invariant in a comment so a future maintainer who edits the stop → create edge in the normal path knows they must also add createNode unconditionally in the remove deps. Co-Authored-By: Claude Opus 4.7 (1M context) Signed-off-by: Guillaume Lours --- pkg/compose/reconcile.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/pkg/compose/reconcile.go b/pkg/compose/reconcile.go index a3f766f66..c3bab7cc2 100644 --- a/pkg/compose/reconcile.go +++ b/pkg/compose/reconcile.go @@ -640,9 +640,16 @@ func (r *reconciler) planRecreateContainer(service types.ServiceConfig, oc *Obse r.stoppedByPlan[oc.ID] = stopNode } - // 3. Remove old container. Depend on the (existing or new) stop and on - // the create node so the new container is in place before the old one - // goes away. + // 3. Remove old container. The invariant is "new container exists before + // old one is removed": + // - !alreadyStopped: stopNode is the one we just created with + // DependsOn=[createNode], so the chain remove → stop → create + // guarantees the invariant transitively. + // - alreadyStopped: stopNode comes from planRecreateNetwork and was + // emitted before createNode. The transitive guarantee no longer + // holds, so we add createNode to removeDeps explicitly. If anyone + // ever drops the stop → create edge from the !alreadyStopped case, + // they must add createNode here unconditionally. removeDeps := []*PlanNode{stopNode} if alreadyStopped { removeDeps = append(removeDeps, createNode)