🐛 fix(release): a refusal names the bundle the operator passed, not the copy

internal/adapters/source/local/local.go

@@ -58,13 +58,16 @@ func (s *Source) Schemes() []string { return []string{Scheme} }
 // one transport verify from another, and is worth the extra unpack of a bundle
 // measured in kilobytes.
 func (s *Source) Resolve(ctx context.Context, ref ports.Ref) (ports.ResolvedRelease, error) {
-	dir, cleanup, err := s.materialise(ref)
+	dir, source, cleanup, err := s.materialise(ref)
 	if err != nil {
 		return ports.ResolvedRelease{}, err
 	}
 	defer cleanup()
 
-	rel, err := release.Load(dir)
+	// Named for the archive the operator passed, not for the directory it
+	// was unpacked into. `source` is empty for a bundle already on disk as
+	// a directory, where the two are the same path anyway.
+	rel, err := release.LoadAs(dir, source)
 	if err != nil {
 		return ports.ResolvedRelease{}, err
 	}
@@ -104,7 +107,22 @@ func (s *Source) Fetch(ctx context.Context, ref ports.Ref, destDir string) (port
 		if err := atomicfs.ExtractTarZst(path, destDir, s.limits); err != nil {
 			return "", err
 		}
-		if _, err := release.Load(destDir); err != nil {
+		// Named for the archive, not for where it was just unpacked. A
+		// plan unpacks into a temporary directory and a real install
+		// into the release store, and neither is a path the operator
+		// chose or can act on -- this is the only read of the bundle a
+		// caller of Fetch does not make itself, so it has to carry the
+		// name too.
+		if _, err := release.LoadAs(destDir, path); err != nil {
+			// Extracted before it could be validated, so a refusal here
+			// is a refusal to a directory this call already filled.
+			// Neither caller cleans it up -- `fetchRelease` returns
+			// straight out on a Fetch error, and `stepStageRelease`'s
+			// compensation keys off the release in engine state, which
+			// a failed Fetch never put there -- and an unusable release
+			// left in the store is one `update --to` away from being
+			// installed by somebody who never saw this error.
+			_ = atomicfs.RemoveAll(destDir)
 			return "", err
 		}
 		return ports.BundlePath(destDir), nil
@@ -172,26 +190,29 @@ func (s *Source) List(ctx context.Context, ref ports.Ref) ([]domain.Version, err
 //
 // The returned cleanup is always non-nil, so callers can defer it
 // unconditionally rather than branching on how the bundle arrived.
-func (s *Source) materialise(ref ports.Ref) (dir string, cleanup func(), err error) {
+// The returned `source` is what the operator named, and is empty when that is
+// the directory being returned -- so a caller can name the archive in an error
+// about its contents without having to know whether one was unpacked.
+func (s *Source) materialise(ref ports.Ref) (dir, source string, cleanup func(), err error) {
 	path, err := s.locate(ref)
 	if err != nil {
-		return "", func() {}, err
+		return "", "", func() {}, err
 	}
 	if !atomicfs.IsTarZst(path) {
-		return path, func() {}, nil
+		return path, "", func() {}, nil
 	}
 
 	tmp, err := os.MkdirTemp("", "morzer-resolve-")
 	if err != nil {
-		return "", func() {}, domain.Internal(err, "cannot create a temporary directory")
+		return "", "", func() {}, domain.Internal(err, "cannot create a temporary directory")
 	}
 	cleanup = func() { _ = atomicfs.RemoveAll(tmp) }
 
 	if err := atomicfs.ExtractTarZst(path, tmp, s.limits); err != nil {
 		cleanup()
-		return "", func() {}, err
+		return "", "", func() {}, err
 	}
-	return tmp, cleanup, nil
+	return tmp, path, cleanup, nil
 }
 
 // locate resolves the reference to a bundle directory or archive file.

internal/adapters/source/local/local_test.go

@@ -6,10 +6,13 @@ import (
 	"path/filepath"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/morzecrew/morzer/internal/adapters/source/local"
 	"github.com/morzecrew/morzer/internal/domain"
+	"github.com/morzecrew/morzer/internal/infra/atomicfs"
 	"github.com/morzecrew/morzer/internal/ports"
+	"github.com/morzecrew/morzer/internal/release"
 )
 
 // This adapter is the reference implementation of ReleaseSource: resolve
@@ -355,3 +358,33 @@ func copyTree(src, dst string) error {
 		return os.WriteFile(target, data, info.Mode().Perm())
 	})
 }
+
+// Fetch extracts before it validates, so a bundle it refuses is a bundle it
+// has already written. Leaving it there puts an unusable release in the store,
+// one `update --to` away from being installed by somebody who never saw the
+// error -- and neither caller cleans it up: `ops.fetchRelease` returns straight
+// out on a Fetch error, and `stepStageRelease`'s compensation keys off the
+// release in engine state, which a failed Fetch never put there.
+func TestFetchRemovesAnArchiveItRefuses(t *testing.T) {
+	src := t.TempDir()
+	if err := os.WriteFile(filepath.Join(src, release.ManifestFileName),
+		[]byte("schema_version: 1\nmetadata:\n  name: demo\n  version: not-a-version\n"),
+		0o644); err != nil {
+		t.Fatal(err)
+	}
+	archive := filepath.Join(t.TempDir(), "demo-1.2.0.tar.zst")
+	if err := atomicfs.WriteTarZst(archive, src,
+		[]string{release.ManifestFileName}, time.Unix(0, 0)); err != nil {
+		t.Fatal(err)
+	}
+
+	dest := filepath.Join(t.TempDir(), "releases", "1.2.0")
+	if _, err := local.New().Fetch(context.Background(), ref(archive), dest); err == nil {
+		t.Fatal("expected the invalid bundle to be refused")
+	}
+
+	entries, err := os.ReadDir(dest)
+	if err == nil && len(entries) > 0 {
+		t.Fatalf("Fetch left %d entries in the destination it refused", len(entries))
+	}
+}

internal/lifecycle/ops/ops.go

@@ -315,7 +315,13 @@ func (d *Deps) checkPlannedRelease(ctx context.Context, releasePath string,
 	}
 	// LoadManifest validates: the answer this plan needs is already computed
 	// here, and used to be discarded with a bare return.
-	m, err := release.LoadManifest(filepath.Join(bundle.String(), release.ManifestFileName))
+	//
+	// Named for what the operator passed, not for the copy above. The real
+	// path names the source without trying, because Resolve reads a local
+	// bundle in place -- so a plan that named its own scratch directory
+	// disagreed with the run about a path they were both refusing.
+	m, err := release.LoadManifestAs(
+		filepath.Join(bundle.String(), release.ManifestFileName), releasePath)
 	if err != nil {
 		return false, err
 	}

internal/release/load.go

@@ -42,27 +42,56 @@ const ReleaseNotesFileName = "RELEASE.md"
 // The digest is computed over the whole tree, so identity is content-based
 // rather than trusting what the manifest claims about itself.
 func Load(dir string) (domain.Release, error) {
+	return loadFrom(dir, "")
+}
+
+// LoadAs is Load naming `source` in whatever it refuses, instead of the
+// directory it actually read.
+//
+// For the same reason LoadManifestAs exists, one level up: a source that has to
+// unpack an archive before it can read the bundle reads an extracted copy, and
+// naming that copy points the operator at a directory they never chose and
+// which is removed on the way out. The archive is the shape a vendor publishes,
+// so that is the install path most operators take.
+//
+// An empty `source` means "name what you read", which is what Load passes. The
+// distinction from LoadManifestAs is deliberate: that one is handed a source by
+// a caller that has decided to override, and an empty string there is a bug,
+// while here the empty case is the un-overridden default and has a caller.
+func LoadAs(dir, source string) (domain.Release, error) {
+	return loadFrom(dir, source)
+}
+
+func loadFrom(dir, source string) (domain.Release, error) {
+	named := func(real string) string {
+		if source != "" {
+			return source
+		}
+		return real
+	}
+
 	abs, err := filepath.Abs(dir)
 	if err != nil {
-		return domain.Release{}, domain.ValidationError(err, "cannot resolve %s", dir)
+		return domain.Release{}, domain.ValidationError(err, "cannot resolve %s", named(dir))
 	}
 
 	info, err := os.Stat(abs)
 	if err != nil {
 		if errors.Is(err, fs.ErrNotExist) {
 			return domain.Release{}, domain.ValidationError(domain.ErrReleaseNotFound,
-				"no release bundle at %s", abs).
+				"no release bundle at %s", named(abs)).
 				WithHint("check the path, or run `morzer release list` to see installed releases")
 		}
-		return domain.Release{}, domain.ValidationError(err, "cannot read %s", abs)
+		return domain.Release{}, domain.ValidationError(err, "cannot read %s", named(abs))
 	}
 	if !info.IsDir() {
 		return domain.Release{}, domain.ValidationError(nil,
-			"%s is not a directory", abs).
+			"%s is not a directory", named(abs)).
 			WithHint("point at an unpacked bundle directory containing %s", ManifestFileName)
 	}
 
-	manifest, err := LoadManifest(filepath.Join(abs, ManifestFileName))
+	manifestPath := filepath.Join(abs, ManifestFileName)
+	manifest, err := LoadManifestAs(manifestPath, named(manifestPath))
 	if err != nil {
 		return domain.Release{}, err
 	}
@@ -95,16 +124,40 @@ func Load(dir string) (domain.Release, error) {
 
 // LoadManifest reads, decodes, defaults and validates one manifest file.
 func LoadManifest(path string) (domain.Manifest, error) {
+	return LoadManifestAs(path, path)
+}
+
+// LoadManifestAs is LoadManifest naming `source` in whatever it refuses,
+// instead of the file it actually read.
+//
+// The two differ only where the bytes came from a copy. A plan stages the
+// bundle into a temporary directory to read it, and every message naming that
+// directory points the operator at a path they never chose and which is gone
+// before they can go and look at it. ParseManifest prefixes the source so an
+// author with several bundles open knows which one is being complained about,
+// and a temp path answers that question with a path they cannot place.
+//
+// Callers reading a file the operator named pass LoadManifest and get the two
+// arguments equal, which is the honest default: naming the file you read is
+// right everywhere except when you read a copy on their behalf.
+//
+// `source` must not be empty, and there is deliberately no fallback for it. An
+// empty one renders as `error: : manifest is invalid:`, which is loud, wrong in
+// a way nobody can mistake for intended, and caught by the assertions in
+// `TestAFirstInstallRefusesADeprecatedBundle`. Quietly substituting `path`
+// would turn a caller's mistake into the exact defect this function exists to
+// remove, and leave it to be found by an operator instead of by a test.
+func LoadManifestAs(path, source string) (domain.Manifest, error) {
 	data, err := os.ReadFile(path)
 	if err != nil {
 		if errors.Is(err, fs.ErrNotExist) {
 			return domain.Manifest{}, domain.ValidationError(domain.ErrReleaseNotFound,
-				"no manifest at %s", path).
+				"no manifest at %s", source).
 				WithHint("every release bundle must contain a %s at its root", ManifestFileName)
 		}
-		return domain.Manifest{}, domain.ValidationError(err, "cannot read %s", path)
+		return domain.Manifest{}, domain.ValidationError(err, "cannot read %s", source)
 	}
-	return ParseManifest(data, path)
+	return ParseManifest(data, source)
 }
 
 // managerVersion is the running manager's own version, recorded once at

logs/wave-39.md

@@ -209,3 +209,48 @@ than the missing value.
 
 **Drift count: still 0.** Both entries are gaps in what this wave built, found
 before it merged, and neither contradicts a document.
+
+## Correction — 2026-08-27, the exit code depends on a flag this entry did not name
+
+**The entry above, and the "Carried into the next unit" item it fed, say the
+real `init` refuses a legacy bundle at exit 11. It does that only when
+`--product` is passed.** The claim is left standing, as this collection's
+corrections are.
+
+Measured on wave 41's branch against a real binary and a legacy directory
+bundle:
+
+```
+morzer init --release <legacy> …                 → exit 2
+morzer init --product demo --release <legacy> …  → exit 11
+morzer init --product demo --release <legacy> … --dry-run → exit 2
+```
+
+Without `--product` the CLI reads the manifest at the source to learn the
+product name (`internal/cli/commands.go:64`), and validation comes with the
+read — so the refusal lands before any operation is built. With it, that read is
+skipped, three steps run and are rolled back, and the failure is the step's.
+
+The shape the entry described is real and worse than it says. On the exit-11
+path the top-level error is `code: compensated, category: system` — a system
+category for what is the operator's input — and the manifest problem appears
+exactly once in the whole `--json` stream, inside `record.steps[3].error`. A
+consumer reading `.error` is told the operation rolled back and nothing about
+why.
+
+**The cause is general, not this command's.** `domain.ExitCode` tests
+`ErrCompensated` (`internal/domain/errors.go:336`) before `ErrUsage` and
+`ErrValidation` (line 354), so the compensation wrapper outranks every cause in
+every compensated operation. Nothing records that precedence as a decision: no
+RFC decision row covers it, `docscheck`'s `checkExitCodes` only asserts each
+constant has a row in the published table, and `internal/domain` has no direct
+test of `ExitCode` at all. A change to that ordering passes every gate in
+`just ci`.
+
+Carried to an RFC rather than fixed. The published table names "an invalid
+manifest" under 2 *and* "back where it started" under 11, and both rows describe
+that run truthfully — which is a question about what an exit code reports, not a
+defect to patch inside a task.
+
+**Drift count: unchanged.** This corrects a claim's scope and names its cause;
+no entry's class changes.