fix(publish): warn when push falls back from OCI 1.1 to OCI 1.0

[htoyoda18]

What I did

PushManifest silently retried in OCI 1.0 format whenever a registry rejected the OCI 1.1 manifest, leaving users unaware their artifact wasn't stored in the newer format. This was left as a TODO because internal/oci intentionally avoids importing logrus.

PushManifest now reports which OCI version was actually used instead of just success/failure, and pkg/compose/publish (which already depends on logrus) logs a warning when that differs from what was requested.

Related issue
N/A

(not mandatory) A picture of a cute animal, if possible in relation to what you did
🐈🐈🐈

[htoyoda18]

Hi @ndeloof @glours 👋 Just a friendly nudge on this one — it's been about a week with no review yet. Happy to address anything if you get a chance to take a look. Thanks!

[Copilot]

🟢 Approval recommended

The change is small and localized, with only minor wording/doc clarity issues noted.

Pull request overview

This PR makes the publish flow transparent when registries reject OCI 1.1 manifests by having internal/oci.PushManifest report which OCI version was ultimately used, and emitting a warning from pkg/compose/publish when an automatic fallback to OCI 1.0 occurs.

Changes:

• Extend internal/oci.PushManifest to return the OCI version used (in addition to the manifest descriptor).
• Update the publish path to log a warning when the default OCI 1.1 push falls back to OCI 1.0.

File summaries

File	Description
pkg/compose/publish.go	Captures the OCI version used during push and warns when falling back to OCI 1.0.
internal/oci/push.go	Returns the OCI version used from PushManifest, including in the fallback path.

Review details

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

---

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

[glours]

PushManifest currently returns the raw OCI version it ended up using (api.OCIVersion), and the caller reconstructs "did we fall back?" by comparing two independent fields:

if options.OCIVersion == "" && usedOCIVersion == api.OCIVersion1_0 {
    logrus.Warn(...)
}

Returning a bool (e.g. didFallback) instead would be a better fit for what's actually needed right now:

• PushManifest is the only place that knows why it picked a given version — the moment it decides to retry with api.OCIVersion1_0 is exactly the fallback event itself. A bool captures that fact directly, at the source, instead of asking the caller to re-derive it from two values that happen to correlate today.
• The caller only ever needs a yes/no signal to decide whether to warn — it never uses the specific api.OCIVersion value for anything else. A richer return type is carrying more information than any consumer reads.
• It removes a hidden coupling: today, if a second fallback tier were ever added (e.g. 1.1 → 1.0 → some legacy format), the caller's usedOCIVersion == api.OCIVersion1_0 check would need to be updated in lockstep to keep detecting it as a fallback — and nothing would force that update, since it'd still compile and just silently stop warning on the new tier. A bool set at the fallback branch itself doesn't have that failure mode.

[htoyoda18]

Thanks for the review, @glours!

I've pushed a fix in 6ddf065:

PushManifest now returns a didFallback bool instead of api.OCIVersion, so the caller no longer needs to re-derive the fallback from two values
Reworded the warning to say the registry "rejected" the OCI 1.1 push rather than "does not support" it (also addresses the Copilot review comment)
Could you take another look? Thanks!

[glours]

pkg/compose/publish.go:133 — logrus.Warn here won't surface in the interactive (TTY) progress renderer: everything else in publish/pushComposeArtifact (e.g. pkg/compose/publish.go:100-104, :125-129) reports status through s.events, and the TTY writer only renders what comes through that bus. As-is, a user running docker compose publish in a normal terminal never sees this warning; only --progress plain/json would show it.

Swapping logrus.Warn for s.events.On(...) on the same repository ID (still at publish.go:133) wouldn't fix it either: publish() fires a Done event for that same ID right after pushComposeArtifact returns (publish.go:100-104), and the TTY model unconditionally overwrites status/text per ID (cmd/display/tty_model.go, taskTree.apply) — so the warning would just get immediately clobbered. I also checked giving it its own event ID: nested as a child, it's never rendered as its own line (children only feed the parent's progress-bar aggregation); as a standalone root row it does display, but it inflates the header to 2/2 and — since the fallback is only known after the push, while repository's Working event fires before it (publish.go:82-86) — always renders below the "published" line, reading backwards.

Suggest threading the fallback bool up to publish() so it's folded into the single terminal event already emitted for repository. This touches lines outside the current diff (publish() at publish.go:94-104, and pushComposeArtifact's signature/returns at publish.go:110-140), so posting as a plain diff rather than inline suggestions:

--- a/pkg/compose/publish.go
+++ b/pkg/compose/publish.go
@@ -91,17 +91,24 @@ func (s *composeService) publish(ctx context.Context, project *types.Project, re
 			fmt.Println(string(indent))
 		}
 	}
+	didFallback := false
 	if !s.dryRun {
-		err = s.pushComposeArtifact(ctx, project, repository, layers, options)
+		didFallback, err = s.pushComposeArtifact(ctx, project, repository, layers, options)
 		if err != nil {
 			return err
 		}
 	}
+	text, status := "published", api.Done
+	if didFallback {
+		text, status = "published (registry rejected OCI 1.1; fell back to OCI 1.0)", api.Warning
+	}
 	s.events.On(api.Resource{
 		ID:     repository,
-		Text:   "published",
-		Status: api.Done,
+		Text:   text,
+		Status: status,
 	})
 	return nil
 }
 
-func (s *composeService) pushComposeArtifact(ctx context.Context, project *types.Project, repository string, layers []v1.Descriptor, options api.PublishOptions) error {
+func (s *composeService) pushComposeArtifact(ctx context.Context, project *types.Project, repository string, layers []v1.Descriptor, options api.PublishOptions) (bool, error) {
 	named, err := reference.ParseDockerRef(repository)
 	if err != nil {
-		return err
+		return false, err
 	}
 	...
 	descriptor, didFallback, err := oci.PushManifest(ctx, resolver, named, layers, options.OCIVersion)
 	if err != nil {
 		s.events.On(api.Resource{
 			ID:     repository,
 			Text:   "publishing",
 			Status: api.Error,
 		})
-		return err
+		return false, err
 	}
-	if didFallback {
-		logrus.Warn("registry rejected the OCI 1.1 artifact push; falling back to OCI 1.0 format")
-	}
-
 	if options.Application {
-		return pushApplicationIndex(ctx, resolver, named, descriptor, project)
+		return didFallback, pushApplicationIndex(ctx, resolver, named, descriptor, project)
 	}
-	return nil
+	return didFallback, nil
 }

[htoyoda18]

Thanks for the review, @glours!

Applied your suggestion: the fallback bool is now threaded up to publish() and folded into the single published event for the repository, instead of a logrus.Warn that never reached the TTY renderer.

One thing to flag: I put the whole message in Text ("published (registry rejected OCI 1.1; fell back to OCI 1.0)"). It works, but Resource already has a Details field for this exact purpose — used elsewhere (e.g. pull.go's Text: "Skipped", Details: ...) and rendered/exposed separately by the TTY and JSON writers. Splitting it that way (Text: "published", Details: "...") would match that convention better. Happy to switch if you agree.

Could you take another look?