plan engine: reconciler plans the start phase (epic #14081, lot 1)

[ndeloof]

What this PR does, in one sentence

It teaches Compose's plan engine to describe starting an application — until now it could only describe creating it — without changing any current behavior: this is vocabulary, and nobody speaks it yet.

Context

Today up works in two steps with two different engines: first the plan engine computes and executes "what to create" (deterministic, testable, comparable against a golden file), then a second, imperative engine improvises the start as it goes — waiting for the database to be healthy, running pre_start hooks, starting containers in dependency order, running post_start. That second engine is neither inspectable nor testable the same way, and the seam between the two is a known source of bugs (two different snapshots of the daemon state, two event-emission systems).

What the PR brings

The plan can now express everything that start phase does: "wait for db to be healthy", "run web's hooks", "start replica 1 then 2", "app only starts once db is ready". The whole start logic becomes data that can be displayed, reviewed and verified before being executed — instead of behavior that only exists at the moment it happens.

Two guardrails frame the PR:

• Inert by construction: the new capability sits behind an option no command passes yet. compose create, up — everything produces exactly the same plans as before, byte for byte.
• Faithful to existing behavior: every subtlety of the current imperative engine is reproduced and locked by tests — dependency conditions are re-verified even when everything is already running, a paused container being restarted gets neither a redundant restart nor a redundant injection, a container doomed to disappear (replica scale-down) is never started, and error cases produce today's messages word for word.

Why this is the right first brick

This is the first item of Lot 1 of epic #14081: lay down the vocabulary and prove its fidelity before wiring anyone onto it. The next PRs will make the executor speak this vocabulary, then switch up, start and scale over one consumer at a time — each staying small and revertible, because the hard part (saying exactly the same thing as the existing engine) will already have been validated here.

---

Split out of #14156 to keep the review focused.

🤖 Generated with Claude Code

[docker-agent]

Assessment: 🟡 NEEDS ATTENTION

2 inline findings (both moderate confidence). No high-severity issues found.

[docker-agent]

Assessment: 🔴 CRITICAL

One high-severity finding in the new start-phase planning logic.

[docker-agent]

Assessment: 🟢 APPROVE

[codecov]

Codecov Report

❌ Patch coverage is 92.03980% with 16 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
pkg/compose/reconcile.go	91.70%	9 Missing and 7 partials ⚠️

📢 Thoughts on this report? Let us know!

[docker-agent]

Assessment: 🟢 APPROVE

The start-phase planning logic is well-structured and the implementation is correct. All hypotheses raised during analysis were dismissed as false positives:

1. seen map in startPhaseReplicas — seen[resID] = true is set unconditionally before the OpStartContainer type check, so exceptional-state containers are correctly excluded from the second loop.
2. service_started on fully-running dependency under ScopeStart — producing no prerequisite edge is the intended behavior and matches the imperative engine's semantics: if the dependency is already running, the service_started condition is trivially satisfied and no ordering constraint is needed. The code comment at the call site makes this explicit.

The inert-by-construction design (gated behind an option no command passes yet), faithful parity with the imperative engine, and comprehensive test coverage are all solid.

[Copilot]

🟡 Changes recommended

Start-phase planning currently treats observed running containers as guaranteed-to-be-running at start execution time, which misses containers that the create phase will stop (e.g., during network recreation), leading to incorrect start plans.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR extends Docker Compose’s plan engine vocabulary so a plan can describe the application Start phase (dependency condition waits, pre_start/post_start hooks, and replica start ordering) in addition to the existing Create phase, while keeping the default behavior inert via a new reconcile scope.

Changes:

• Introduces ReconcileScope and start-phase planning (planStartPhase) that emits start-phase nodes and dependency-condition wait nodes (deduplicated per service+condition).
• Adds PlanNode.Phase (Create vs Start) and new operation types (OpWaitCondition, OpRunPreStart, OpRunPostStart), with plan rendering marking start-phase nodes.
• Adds unit tests that golden-check start-phase planning behavior across dependencies, hooks, replica ordering, and scope modes.

File summaries

File	Description
pkg/compose/reconcile.go	Adds reconcile scope selection and start-phase plan construction (replica chains, deduped waits, hooks), plus new reconciler tracking maps.
pkg/compose/plan.go	Adds start-phase operation types and PlanPhase to tag nodes, and updates plan string rendering to mark start nodes.
pkg/compose/reconcile_start_test.go	Adds coverage to lock down start-phase plan output and semantics across key scenarios.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 3
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

The start-replica ordering comparator in startPhaseReplicas uses subtraction and can overflow, risking incorrect ordering on some platforms or extreme replica numbers.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Lite

[docker-agent]

Assessment: 🟡 NEEDS ATTENTION

Two medium-severity findings in the newly-introduced start-phase planning code. Both are LIKELY (moderate confidence, 67/100) and concern correctness of the DAG-building logic under specific conditions.

[Copilot]

🟢 Approval recommended

The change is gated behind a new scope (default remains create-only) and is backed by focused unit tests, with only a minor maintainability nit noted.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Lite

[docker-agent]

Assessment: 🟢 APPROVE

The plan engine's start-phase extension is well-structured and carefully reasoned. The new code is inert by construction (gated behind an option no existing command passes), and the comprehensive test suite locks every subtlety of the existing imperative engine's behavior.

Key design points reviewed and found sound:

• Dependency ordering: visitInDependencyOrder correctly traverses leaves-first so startChainEnds[dep] is always populated before any dependent is visited.
• Replica deduplication: the seen map correctly gates the second loop over containerNodes, preventing double-counting of observed+planned replicas.
• Exceptional-state replicas: create-phase bare restarts (paused, dead) are correctly counted as anyRunning and excluded from start-phase replicas, matching the imperative engine's second-snapshot behavior.
• Pre_start scoping: the !anyRunning gate faithfully reproduces the imperative engine — pre_start is skipped when any replica is already running.
• Scale-down safety: removedByPlan correctly prevents condemned replicas from receiving start-phase nodes.
• Wait node deduplication: shared OpWaitCondition nodes correctly merge prerequisites and propagate Required upgrades.
• Short-circuit safety: the waitless condition in startPhaseDependencies correctly short-circuits on err != nil before accessing depService fields.
• Replica start ordering: numeric sort via cmp.Compare(a.number, b.number) is correct and lexicography-safe.

[Copilot]

🟡 Changes recommended

There is at least one correctness/maintainability issue to address (an exported-type comment mismatch that can mislead readers and may trip exported-comment lint rules).

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟢 Approval recommended

The start-phase planning is gated behind an explicit scope option (default remains byte-identical) and the new behavior is thoroughly unit-tested for intended semantics and parity.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 0 new
• Review effort level: Lite

[docker-agent]

Assessment: 🟢 APPROVE

The start-phase planning logic is well-structured and faithfully reproduces the imperative engine's semantics. The new planStartPhase / planServiceStart code correctly handles:

• Dependency conditions (service_started as plain edges, others as deduplicated OpWaitCondition nodes)
• Replica ordering (numeric sort with cmp.Compare, tested with 11 replicas)
• Scale-down avoidance (removedByPlan correctly gates condemned replicas)
• Exceptional-state containers (counted as running for pre_start gating, excluded from start-phase replicas)
• Stopped-by-plan containers (network recreate path, ordered after reconnect)
• Scope guard (inert by default, CreateStart/Start scopes opt in)

Two minor points are noted below.

[docker-agent]

Assessment: 🟢 APPROVE

The start-phase plan vocabulary is well-structured and carefully guarded behind ReconcileScope — no existing command passes a non-zero scope, so today's behavior is byte-identical (verified by TestPlanStart_DefaultScopeIsInert). The implementation faithfully reproduces every imperative-engine subtlety: dependency-condition re-evaluation even when everything is running, exceptional-state replicas counting as running to gate pre_start, scale-down victims excluded from the start phase, and numeric replica ordering to avoid lexicographic bugs with 10+ replicas.

The new test file covers all the critical invariants — paused containers, recreate chains, network-stopped-then-restarted containers, optional vs. required conditions, and the error path for missing containers under ScopeStart.

[Copilot]

🟢 Approval recommended

The changes are inert by default (ScopeCreate remains unchanged) and the new Start-phase planning behavior is well covered by targeted unit tests.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 0 new
• Review effort level: Lite

[docker-agent]

Assessment: 🟢 APPROVE

This PR carefully implements start-phase planning for the Compose plan engine. The implementation is faithful to the imperative engine's semantics, well-documented, and thoroughly tested.

What was reviewed:

• planStartPhase / planServiceStart: dependency ordering, replica chaining, pre/post-start hook sequencing
• startPhaseReplicas: all container-state branches (running, exceptional-state, stopped-by-plan, scale-down condemned), numeric sort via cmp.Compare
• startPhaseDependencies / waitConditionNode: condition deduplication, BestEffort upgrade, target merging
• plannedReplica: type switch correctness, unresolved-target default
• All 12 new test cases: logic verified against the code

Notable strengths:

• The nested containerNodes map[string]map[int]*PlanNode index (replacing the previous flat-map approach) eliminates the O(services × replicas) scan
• slices.Clone(prev) before the pre_start block correctly prevents backing-array aliasing
• cmp.Compare(a.number, b.number) for replica ordering is correct and overflow-safe
• All previously identified issues from earlier review cycles have been resolved