fix(build): an unlabelled release image is adopted only from a pinned release

docs/commands.md

@@ -4095,7 +4095,7 @@ move with its code:
 | records this build's code; its recipe or bases differ (another checkout, another builder, a re-pushed base) | adopts it — it is the release's build — and warns naming the class that differs |
 | records other code | refuses: the code changed without its version moving, and the fix is a new release, never a rebuild under the old version |
 | records another agent than the release pins | refuses: the tag holds a build the release does not name (made under an [agent override](#moving-the-agent-version-of-composed-services) by a CLI that pushed it under the release tag) |
-| records only the aggregate (labelled before the classes existed) | adopts it with a warning that its code was not compared; `--rebuild` builds it again instead |
+| records only the aggregate (labelled before the classes existed) | adopts it, with a warning, only when the service builds from a pinned release the CLI materialized (its code is the release's by construction); from a checkout or an override it refuses, since nothing can compare the code — build from the pinned release, or release a new version |
 
 Adopting is the default for an image that cannot be compared because the two
 alternatives are worse: refusing blocks every render of every module on a cell

pkg/composition/materialize.go

@@ -14,6 +14,8 @@ import (
 	"sync"
 	"time"
 
+	"github.com/codefly-dev/cli/pkg/modulecache"
+
 	"github.com/Masterminds/semver"
 	"github.com/codefly-dev/cli/pkg/cli"
 	"github.com/codefly-dev/core/resources"
@@ -691,22 +693,10 @@ func (roots moduleCacheRoots) holds(path string) bool {
 	return underAnyDir([]string{roots.clone, roots.verified}, path)
 }
 
-// verifiedPinnedModuleCacheRoot is the content-addressed module cache the core
-// Materializer writes into. By default it is workspace-scoped — the digest tree
-// NewMaterializer(workspaceDir) picks — while the git-clone cache root is
-// process-global (pinnedModuleCacheRoot); both are recognized when deciding
-// whether an overlay path is CLI-managed. A configured ModuleCacheEnv moves it
-// beside the clones, under the one reserved subdirectory, so a workspace
-// repository holds no module bytes at all.
+// verifiedPinnedModuleCacheRoot is the verified-package cache root
+// (modulecache.VerifiedRoot).
 func verifiedPinnedModuleCacheRoot(workspaceDir string) (string, error) {
-	configured, err := configuredModuleCacheRoot()
-	if err != nil {
-		return "", err
-	}
-	if configured != "" {
-		return filepath.Join(configured, verifiedPackageCacheDirName), nil
-	}
-	return filepath.Join(workspaceDir, ".codefly", "cache", "modules"), nil
+	return modulecache.VerifiedRoot(workspaceDir)
 }
 
 // pruneStalePinnedEntries drops auto-managed cache entries whose module is no
@@ -1130,20 +1120,12 @@ func gitCommand(ctx context.Context, args ...string) *exec.Cmd {
 	return cmd
 }
 
-// ModuleCacheEnv relocates the module cache — both the git-clone cache and the
-// verified-package cache — to a directory the developer chooses, so the modules
-// a workspace composes can be browsed (and deleted) where they expect them
-// rather than inside CODEFLY_HOME and inside the workspace respectively. It
-// must name an absolute path; a leading "~" is expanded.
-const ModuleCacheEnv = "CODEFLY_MODULE_CACHE"
-
-// verifiedPackageCacheDirName is the subdirectory of a configured cache root
-// that holds verified module packages. Those are content-addressed by digest
-// (that addressing is what makes a cache hit checkable against what was
-// verified), so they cannot share the browsable <owner>/<repo>/<version> tree
-// the git clones occupy; keeping them under one reserved name is what lets both
-// live under the single root the developer configured.
-const verifiedPackageCacheDirName = ".packages"
+// ModuleCacheEnv relocates the module cache (modulecache.Env).
+const ModuleCacheEnv = modulecache.Env
+
+// verifiedPackageCacheDirName is the verified packages' subdirectory of a
+// configured cache root (modulecache.VerifiedDirName).
+const verifiedPackageCacheDirName = modulecache.VerifiedDirName
 
 // stagingCacheDirName is the subdirectory of a cache root that holds in-flight
 // clones. They live under one reserved name rather than as `.pull-*` siblings of
@@ -1153,53 +1135,14 @@ const verifiedPackageCacheDirName = ".packages"
 // the checkouts it is promoted into, which being under the same root guarantees.
 const stagingCacheDirName = ".staging"
 
-// configuredModuleCacheRoot returns the root named by ModuleCacheEnv, or "" when
-// it is unset. A value that is not usable as a root is an error rather than an
-// ignored setting: silently falling back would put the modules somewhere other
-// than where the developer said, and they would look for them where they asked.
-func configuredModuleCacheRoot() (string, error) {
-	raw := strings.TrimSpace(os.Getenv(ModuleCacheEnv))
-	if raw == "" {
-		return "", nil
-	}
-	expanded := raw
-	if raw == "~" || strings.HasPrefix(raw, "~/") {
-		home, err := os.UserHomeDir()
-		if err != nil {
-			return "", fmt.Errorf("%s is %q but the home directory cannot be resolved: %w", ModuleCacheEnv, raw, err)
-		}
-		expanded = filepath.Join(home, strings.TrimPrefix(strings.TrimPrefix(raw, "~"), "/"))
-	}
-	if !filepath.IsAbs(expanded) {
-		return "", fmt.Errorf("%s must be an absolute path (or start with ~), got %q", ModuleCacheEnv, raw)
-	}
-	return filepath.Clean(expanded), nil
-}
+// configuredModuleCacheRoot is modulecache.Configured.
+func configuredModuleCacheRoot() (string, error) { return modulecache.Configured() }
 
-// pinnedModuleCacheRoot is the root of the git-clone cache: the configured root
-// when there is one, otherwise the process-global default. Under either, a
-// module lands at <root>/<owner>/<repo>/<tag>/ plus its optional module
-// subpath, which is the layout a developer pointing ModuleCacheEnv at their own
-// vendor directory browses.
-func pinnedModuleCacheRoot() (string, error) {
-	configured, err := configuredModuleCacheRoot()
-	if err != nil {
-		return "", err
-	}
-	if configured != "" {
-		return configured, nil
-	}
-	return filepath.Join(resources.CodeflyHomeDir(), "modules"), nil
-}
+// pinnedModuleCacheRoot is the git-clone cache root (modulecache.CloneRoot).
+func pinnedModuleCacheRoot() (string, error) { return modulecache.CloneRoot() }
 
 // underDir reports whether path is dir itself or nested inside it.
-func underDir(dir, path string) bool {
-	rel, err := filepath.Rel(dir, path)
-	if err != nil {
-		return false
-	}
-	return rel == "." || (!strings.HasPrefix(rel, ".."+string(filepath.Separator)) && rel != "..")
-}
+func underDir(dir, path string) bool { return modulecache.Under(dir, path) }
 
 // underAnyDir reports whether path is under any of dirs (empty entries are
 // skipped). Used to recognize both the git-clone fallback's cache root and the

pkg/modulecache/modulecache.go

@@ -0,0 +1,118 @@
+// Package modulecache names where the CLI materializes the modules a
+// workspace pins, and answers whether a path is one it wrote. It is a leaf so
+// that both the composition that materializes modules and the builder that
+// must know whether a service's code is a pinned release can read one answer.
+package modulecache
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
+
+	"github.com/codefly-dev/core/resources"
+)
+
+// Env relocates the module cache — both the git-clone cache and the
+// verified-package cache — to a directory the developer chooses, so the modules
+// a workspace composes can be browsed (and deleted) where they expect them
+// rather than inside CODEFLY_HOME and inside the workspace respectively. It
+// must name an absolute path; a leading "~" is expanded.
+const Env = "CODEFLY_MODULE_CACHE"
+
+// VerifiedDirName is the subdirectory of a configured cache root that holds
+// verified module packages. Those are content-addressed by digest (that
+// addressing is what makes a cache hit checkable against what was verified),
+// so they cannot share the browsable <owner>/<repo>/<version> tree the git
+// clones occupy; keeping them under one reserved name is what lets both live
+// under the single root the developer configured.
+const VerifiedDirName = ".packages"
+
+// Configured returns the root named by Env, or "" when it is unset. A value
+// that is not usable as a root is an error rather than an ignored setting:
+// silently falling back would put the modules somewhere other than where the
+// developer said, and they would look for them where they asked.
+func Configured() (string, error) {
+	raw := strings.TrimSpace(os.Getenv(Env))
+	if raw == "" {
+		return "", nil
+	}
+	expanded := raw
+	if raw == "~" || strings.HasPrefix(raw, "~/") {
+		home, err := os.UserHomeDir()
+		if err != nil {
+			return "", fmt.Errorf("%s is %q but the home directory cannot be resolved: %w", Env, raw, err)
+		}
+		expanded = filepath.Join(home, strings.TrimPrefix(strings.TrimPrefix(raw, "~"), "/"))
+	}
+	if !filepath.IsAbs(expanded) {
+		return "", fmt.Errorf("%s must be an absolute path (or start with ~), got %q", Env, raw)
+	}
+	return filepath.Clean(expanded), nil
+}
+
+// CloneRoot is the root of the git-clone cache: the configured root when there
+// is one, otherwise the process-global default. Under either, a module lands
+// at <root>/<owner>/<repo>/<tag>/ plus its optional module subpath, which is
+// the layout a developer pointing Env at their own vendor directory browses.
+func CloneRoot() (string, error) {
+	configured, err := Configured()
+	if err != nil {
+		return "", err
+	}
+	if configured != "" {
+		return configured, nil
+	}
+	return filepath.Join(resources.CodeflyHomeDir(), "modules"), nil
+}
+
+// VerifiedRoot is the content-addressed module cache the core Materializer
+// writes into. By default it is workspace-scoped — the digest tree
+// NewMaterializer(workspaceDir) picks — while the git-clone cache root is
+// process-global (CloneRoot). A configured Env moves it beside the clones,
+// under the one reserved subdirectory, so a workspace repository holds no
+// module bytes at all.
+func VerifiedRoot(workspaceDir string) (string, error) {
+	configured, err := Configured()
+	if err != nil {
+		return "", err
+	}
+	if configured != "" {
+		return filepath.Join(configured, VerifiedDirName), nil
+	}
+	return filepath.Join(workspaceDir, ".codefly", "cache", "modules"), nil
+}
+
+// Owned reports whether path lies under a cache root that holds nothing but
+// what the CLI materialized for workspaceDir: a pinned module's code exactly as
+// its release carries it. A root the developer configured through Env is never
+// proof — it is a directory they chose and may keep their own checkouts in.
+func Owned(workspaceDir, path string) (bool, error) {
+	configured, err := Configured()
+	if err != nil || configured != "" {
+		return false, err
+	}
+	clone, err := CloneRoot()
+	if err != nil {
+		return false, err
+	}
+	verified, err := VerifiedRoot(workspaceDir)
+	if err != nil {
+		return false, err
+	}
+	for _, root := range []string{clone, verified} {
+		if Under(root, path) {
+			return true, nil
+		}
+	}
+	return false, nil
+}
+
+// Under reports whether path is dir itself or nested inside it.
+func Under(dir, path string) bool {
+	rel, err := filepath.Rel(dir, path)
+	if err != nil {
+		return false
+	}
+	return rel == "." || (!strings.HasPrefix(rel, ".."+string(filepath.Separator)) && rel != "..")
+}

pkg/modulecache/modulecache_test.go

@@ -0,0 +1,32 @@
+package modulecache
+
+import (
+	"path/filepath"
+	"testing"
+)
+
+// Only a root that holds nothing but CLI output proves a path is a pinned
+// release; a checkout beside it, or any path under a configured root, does not.
+func TestOwnedIsOnlyTheCLIsOwnRoots(t *testing.T) {
+	workspace := t.TempDir()
+	t.Setenv(Env, "")
+	clone, err := CloneRoot()
+	if err != nil {
+		t.Fatal(err)
+	}
+	for path, want := range map[string]bool{
+		filepath.Join(clone, "acme", "module-x", "v1.2.3", "services", "api"):    true,
+		filepath.Join(workspace, ".codefly", "cache", "modules", "sha256", "ab"): true,
+		filepath.Join(workspace, "modules", "x", "services", "api"):              false,
+		filepath.Join(filepath.Dir(clone), "modules-checkout", "x"):              false,
+	} {
+		if got, err := Owned(workspace, path); err != nil || got != want {
+			t.Errorf("Owned(%s) = %v, %v, want %v", path, got, err, want)
+		}
+	}
+	t.Setenv(Env, t.TempDir())
+	configured, _ := Configured()
+	if got, err := Owned(workspace, filepath.Join(configured, "acme", "module-x", "v1.2.3")); err != nil || got {
+		t.Errorf("a configured root proved ownership: %v, %v", got, err)
+	}
+}

pkg/orchestration/build_plan.go

@@ -14,6 +14,8 @@ import (
 	"strings"
 	"time"
 
+	"github.com/codefly-dev/cli/pkg/modulecache"
+
 	"google.golang.org/protobuf/proto"
 
 	"github.com/codefly-dev/cli/pkg/environments"
@@ -244,7 +246,11 @@ func (b *Builder) judgePublishedRelease(ctx context.Context, w *wool.Wool, recip
 		// other inputs.
 		return nil, w.Wrap(publishedFromOtherInputs(recipe.GetImage(), b.instance.Service.Agent, labels[buildAgentLabel]))
 	case identity.Key != "" && published != "":
-		warning, refusal := publishedReleaseVerdict(recipe.GetImage(), identity, b.instance.Service.Agent, labels)
+		pinned, err := modulecache.Owned(b.world.Workspace.Dir(), b.instance.Service.Dir())
+		if err != nil {
+			return nil, w.Wrapf(err, "cannot tell whether %s builds a pinned release", b.instance.Unique())
+		}
+		warning, refusal := publishedReleaseVerdict(recipe.GetImage(), identity, b.instance.Service.Agent, labels, pinned)
 		if refusal != nil {
 			return nil, w.Wrap(refusal)
 		}
@@ -1127,9 +1133,12 @@ func withBuildInputsLabel(args []string, identity imageIdentity) []string {
 // override (override_build.go), which the release does not name.
 //
 // An image labelled before the classes were recorded carries only the
-// aggregate, so its code cannot be compared: it is adopted, saying so, and
-// --rebuild is the operator's way to build it again instead.
-func publishedReleaseVerdict(image string, identity imageIdentity, agent *resources.Agent, labels map[string]string) (string, error) {
+// aggregate, so its code cannot be compared with this build's. It is adopted
+// only when pinned says this build's code is a pinned release the CLI
+// materialized (modulecache.Owned) — the release's own code, by construction —
+// and refused otherwise: from a checkout, an edited service whose version did
+// not move would be handed the earlier release's image with nothing compared.
+func publishedReleaseVerdict(image string, identity imageIdentity, agent *resources.Agent, labels map[string]string, pinned bool) (string, error) {
 	const release = "release a new version (codefly publish) instead of rebuilding a published one"
 	publishedAgent := labels[buildAgentLabel]
 	if agent != nil && publishedAgent != "" && publishedAgent != agent.Identifier() {
@@ -1141,9 +1150,12 @@ func publishedReleaseVerdict(image string, identity imageIdentity, agent *resour
 	}
 	code := labels[buildCodeLabel]
 	switch {
+	case code == "" && pinned:
+		return "the published image predates per-class build labels; this build's code is the pinned release the CLI materialized, " +
+			"so adopting it as the release's build (--rebuild builds it again)", nil
 	case code == "":
-		return "the published image predates per-class build labels, so its code was not compared with this build's; " +
-			"adopting it as the release's build (--rebuild builds it again)", nil
+		return "", fmt.Errorf("%s is already published, and it predates per-class build labels, so its code cannot be compared with this build's, "+
+			"which is not a pinned release the CLI materialized (a checkout or an override): build from the pinned release, or %s", image, release)
 	case code != classes.Code:
 		return "", fmt.Errorf("%s is already published from other code: the service's build context or Go module manifests differ from the published build's, "+
 			"so its code changed without its version moving; %s", image, release)

pkg/orchestration/image_cache_test.go

@@ -9,6 +9,8 @@ import (
 	"testing"
 	"time"
 
+	"github.com/codefly-dev/cli/pkg/modulecache"
+
 	"github.com/codefly-dev/cli/pkg/environments"
 	coreservices "github.com/codefly-dev/core/agents/services"
 	builderv0 "github.com/codefly-dev/core/generated/go/codefly/services/builder/v0"
@@ -856,17 +858,34 @@ func TestAPublishedVersionFromTheSameInputsIsAdoptedNotRebuilt(t *testing.T) {
 }
 
 // An image labelled before builds recorded their input classes carries only the
-// aggregate, which cannot say whether its code is this build's. It is the
-// release's build, so it is adopted, not rebuilt over and not refused.
-func TestAPublishedVersionLabelledBeforeClassesIsAdopted(t *testing.T) {
+// aggregate, which cannot say whether its code is this build's. Built from the
+// pinned release the CLI materialized, the code is the release's by
+// construction, so the image is adopted, not rebuilt over and not refused.
+func TestAPublishedVersionLabelledBeforeClassesIsAdoptedFromAPinnedRelease(t *testing.T) {
 	harness := newImageCacheHarness(t, true, true)
+	// The clone root is <CODEFLY_HOME>/modules, which holds the service.
+	t.Setenv(resources.CodeflyHomeEnv, harness.workspaceDir)
+	t.Setenv(modulecache.Env, "")
 	harness.peer.image = "example.test/app:1.2.3"
 	published := "sha256:" + strings.Repeat("d", 64)
 	stubPublishedImage(t, published, "sha256:"+strings.Repeat("e", 64))
 	require.Equal(t, published, harness.build(t))
 	require.Equal(t, 0, harness.docker.calls(t, "build"))
 }
 
+// From a checkout, the same image is refused: an edited service whose version
+// did not move would otherwise be handed the earlier release's image with
+// nothing compared.
+func TestAPublishedVersionLabelledBeforeClassesIsRefusedFromACheckout(t *testing.T) {
+	harness := newImageCacheHarness(t, true, true)
+	harness.peer.image = "example.test/app:1.2.3"
+	stubPublishedImage(t, "sha256:"+strings.Repeat("d", 64), "sha256:"+strings.Repeat("e", 64))
+	ctx, cancel := context.WithTimeout(t.Context(), time.Minute)
+	defer cancel()
+	require.ErrorContains(t, harness.tryBuild(ctx, t), "cannot be compared with this build's")
+	require.Equal(t, 0, harness.docker.calls(t, "build"), "a published release is never rebuilt")
+}
+
 // An image published before builds recorded their inputs proves nothing either
 // way, so it is built again, as it always was.
 func TestAPublishedVersionWithNoInputRecordIsBuiltAgain(t *testing.T) {