[ndeloof]

What this PR does, in one sentence

Demo, not meant to be merged: shows how getRestartPolicy shrinks to reading restart alone once compose-go merges deploy.restart_policy into it and rejects contradictory values (compose-spec/compose-go#989).

Context

Today the restart policy of a container is computed in two steps in getRestartPolicy: from restart, then replaced as a whole by deploy.restart_policy when that one is set. That replacement is the source of #14383 (a restart_policy without condition, a delay alone for instance, silently turns restart: always into a container that never restarts), and #14459 fixes it by merging the two field by field in Compose. Contradictory values are still settled without a word.

What the PR shows

• go.mod points compose-go at the branch of loader: reconcile restart and deploy.restart_policy, reject contradictions compose-spec/compose-go#989 (a replace to the fork, which is what keeps this PR a demo).
• With it, the loader has already merged deploy.restart_policy into restart (condition is the name of the policy, max_attempts its retry count) and refused a service whose two attributes contradict each other. getRestartPolicy is then a single read of restart, without the second step and without any merge rule of its own.
• TestGetRestartPolicy loads real compose YAML and checks the policy handed to the engine: the delay-only case of [BUG] deploy.restart_policy without condition drops the restart policy #14383, the two attributes completing each other, and the contradiction reported at load time.
• docker compose run keeps working as is: it already clears both restart and deploy.restart_policy for the one-off container.

Things to know before taking this further

• A service that only had deploy.restart_policy.condition now carries a restart value, so its config hash changes once and the container is recreated on the first up after the upgrade.
• Services built by hand through the SDK, which do not go through the loader, would no longer get their deploy.restart_policy read. The merge in compose (fix: merge deploy.restart_policy fields instead of replacing restart #14459) stays the right fix until compose-go#989 is released, and decides whether Compose keeps a fallback for such projects.
• A service that sets both attributes with different policies is rejected at load time, instead of one of them being dropped silently.

🤖 Generated with Claude Code

[docker-agent]

Assessment: 🟢 APPROVE

[docker-agent]

Assessment: 🟡 NEEDS ATTENTION

This demo PR correctly simplifies getRestartPolicy to a single read of service.Restart, trusting the forked compose-go loader to have already merged deploy.restart_policy. The logic is clean and the new tests (using t.Context(), loading real YAML, covering all the cases from #14383) are well-structured.

One confirmed concern: the simplified function has an implicit hard dependency on the fork's merge behavior — without it, services that specify only deploy.restart_policy silently receive an empty restart policy. The PR description documents this, but a tracking marker in the code would help prevent accidental merge before the upstream PR is released.

Dismissed findings (not posted inline)

• [low] pkg/compose/create_test.go:572 — Test cases validate fork-specific behavior (by design; test is correct for the demo context)

[docker-agent]

[medium] getRestartPolicy silently drops deploy.restart_policy without the forked compose-go

The simplified function trusts that service.Restart already incorporates deploy.restart_policy, but that merge is performed by the forked loader (github.com/ndeloof/compose-go/v2). If this PR is merged — or if the replace directive is later removed without the upstream compose-spec/compose-go#989 also being merged — any service that specifies only deploy.restart_policy (no restart: key) will silently receive an empty container.RestartPolicy{}, meaning the container runs with no restart policy at all.

The PR description explicitly notes this risk: "Services built by hand through the SDK, which do not go through the loader, would no longer get their deploy.restart_policy read." The same applies at the Go-module level if the fork is ever swapped back for the official module without the upstream PR.

This is an inherent design constraint of the demo. The comment at lines 751–753 documents the assumption clearly, which helps a future reviewer catch it. Consider adding a //nolint or build-tag guard to prevent accidental merge, or at a minimum add a TODO(compose-spec/compose-go#989) marker so the coupling is tracked.

Confidence	Score
🟢 strong	100/100

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[docker-agent]

Assessment: 🟢 APPROVE

Incremental review — this review covers only the commits pushed since b31804a9aef3. Earlier commits were reviewed in a prior cycle.

The incremental diff is a single-line update to the replace directive in go.mod, bumping the compose-go fork pseudo-version from v2.0.0-20261105081003-860ac4acaec0 to v2.0.0-20261105145003-9652c7ff3592. The new pseudo-version is syntactically correct (proper YYYYMMDDHHMMSS timestamp, 12-char hash prefix). No logic, security, or integrity bugs are introduced.

Lower-confidence findings (not posted inline)

• [low] go.mod:144 — Replace directive uses a pseudo-version with a 12-character hash prefix, not a full 40-char SHA (confidence: unverified / low severity). Go pseudo-versions intentionally use a truncated hash — this is correct module format. The practical concern is availability: if the upstream fork is rebased or the branch force-pushed, the exact commit may become unreachable. As a demo PR not intended for merge, the risk is minimal and expected.