localenv: keep every pre-run pyproject.toml via timestamped backups

libs/localenv/pipeline.go

@@ -24,6 +24,10 @@ const (
 	venvDir    = ".venv"
 )
 
+// backupTimestampLayout is the UTC, second-resolution stamp in a timestamped
+// backup name (pyproject.toml.<stamp>.bak); shared with tests that pin the name.
+const backupTimestampLayout = "20060102T150405Z"
+
 // artifactSource values reported in --json resolved.artifactSource (spec §6).
 const (
 	artifactNetwork = "network"
@@ -60,6 +64,18 @@ type Pipeline struct {
 
 	// res accumulates phase statuses and result fields as the run progresses.
 	res *Result
+
+	// nowFn supplies the time for backup filenames; nil means time.Now. Injected
+	// in tests so backup names are deterministic.
+	nowFn func() time.Time
+}
+
+// clock returns the current time, using nowFn when injected.
+func (p *Pipeline) clock() time.Time {
+	if p.nowFn != nil {
+		return p.nowFn()
+	}
+	return time.Now()
 }
 
 // Run executes all pipeline phases in order and returns a fully populated Result.
@@ -271,19 +287,67 @@ func (p *Pipeline) pyprojectPath() string {
 	return filepath.Join(p.ProjectDir, pyprojectFile)
 }
 
-// backupPath returns the path to the pyproject.toml backup file.
+// backupPath returns the path to the canonical pyproject.toml backup file.
 func (p *Pipeline) backupPath() string {
 	return filepath.Join(p.ProjectDir, backupFile)
 }
 
+// timestampedBackupBase is the <projectDir>/pyproject.toml.<UTC timestamp> stem a
+// non-first backup name is built from.
+func (p *Pipeline) timestampedBackupBase() string {
+	return filepath.Join(p.ProjectDir, pyprojectFile+"."+p.clock().UTC().Format(backupTimestampLayout))
+}
+
+// backupCurrent writes content to a backup of pyproject.toml and returns its
+// path, never overwriting an existing backup (invariant 2). The canonical
+// pyproject.toml.bak is written once and kept as the pristine original; later
+// backups are pyproject.toml.<timestamp>.bak. mode is preserved onto the file.
+func (p *Pipeline) backupCurrent(content []byte, mode os.FileMode) (string, error) {
+	canonical := p.backupPath()
+	switch err := writeNew(canonical, content, mode); {
+	case err == nil:
+		return canonical, nil
+	case !errors.Is(err, os.ErrExist):
+		return "", err
+	}
+
+	base := p.timestampedBackupBase()
+	candidate := base + ".bak"
+	for i := 1; ; i++ {
+		// Only a name collision advances the suffix, so the loop terminates.
+		switch err := writeNew(candidate, content, mode); {
+		case err == nil:
+			return candidate, nil
+		case !errors.Is(err, os.ErrExist):
+			return "", err
+		}
+		candidate = fmt.Sprintf("%s-%d.bak", base, i)
+	}
+}
+
+// plannedBackupName previews the backup name a real run would create — canonical
+// .bak if none exists yet, else a timestamped name — for --dry-run only. Best-effort:
+// an unstattable .bak returns an error the caller treats as "nothing to report".
+func (p *Pipeline) plannedBackupName() (string, error) {
+	canonical := p.backupPath()
+	_, statErr := os.Stat(canonical)
+	switch {
+	case errors.Is(statErr, os.ErrNotExist):
+		return canonical, nil
+	case statErr == nil:
+		return p.timestampedBackupBase() + ".bak", nil
+	default:
+		return "", statErr
+	}
+}
+
 // mergePlan computes the merged pyproject.toml bytes (without writing to disk),
 // decides greenfield vs. existing, and builds the Plan (populated only under
 // --dry-run). dbcPin is the databricks-connect pin to inject, or "" in
 // constraints-only mode. envVersion is the serverless environment version to
 // write into [tool.databricks.environment], or "" for a cluster target.
 func (p *Pipeline) mergePlan(_ context.Context, pyMinor string, c *Constraints, dbcPin, envVersion string) (merged []byte, greenfield bool, err error) {
 	pyproject := p.pyprojectPath()
-	backup := p.backupPath()
 
 	// The merge base is the live pyproject.toml, not the backup. MergeManaged
 	// rewrites only the three managed regions and preserves every other byte, and
@@ -367,58 +431,52 @@ func (p *Pipeline) mergePlan(_ context.Context, pyMinor string, c *Constraints,
 			ChangedRegions:     changedRegions,
 			WouldInstallPython: pyMinor,
 		}
-		// Report a backup only when a real run would actually write one: for an
-		// existing project with no .bak yet. On a re-run the .bak already exists and
-		// applyMerge keeps it (does not re-write), so claiming a backup here would
-		// describe a write that won't happen.
-		if !greenfield {
-			if _, statErr := os.Stat(backup); errors.Is(statErr, os.ErrNotExist) {
-				plan.WouldBackup = filepath.ToSlash(backup)
+		// Report a backup only when the run would actually write one (i.e. it changes
+		// the file); a no-op re-run writes none.
+		if !greenfield && !bytes.Equal(merged, baseBytes) {
+			if backupName, statErr := p.plannedBackupName(); statErr == nil {
+				plan.WouldBackup = filepath.ToSlash(backupName)
 			}
 		}
 		p.res.Plan = plan
 	}
 	return merged, greenfield, nil
 }
 
-// applyMerge writes the merged bytes to disk, backing up an existing
-// pyproject.toml first. From this point on, disk has been mutated.
+// applyMerge writes the merged bytes to disk, backing up the current
+// pyproject.toml first. From the backup copy onward, disk has been mutated.
 func (p *Pipeline) applyMerge(_ context.Context, mergedBytes []byte, greenfield bool) error {
 	pyproject := p.pyprojectPath()
-	backup := p.backupPath()
 
 	if !greenfield {
-		// Back up before modifying so the user's original is recoverable
-		// (invariant 2). Only create the backup when one does not already exist:
-		// on a re-run the existing .bak is the canonical original unmanaged state
-		// (mergePlan used it as the base), so overwriting it with the already-merged
-		// pyproject.toml would destroy that baseline.
-		_, statErr := os.Stat(backup)
-		switch {
-		case statErr == nil:
-			// Backup already exists — keep it as the canonical baseline.
-		case errors.Is(statErr, os.ErrNotExist):
-			// copyFile creates/truncates the backup path, so a failure mid-copy may
-			// leave a partial .bak: report disk as mutated.
-			if err := copyFile(pyproject, backup); err != nil {
-				return p.fail(PhaseMerge, true, NewError(ErrMerge, err, "backup pyproject.toml failed"))
-			}
-		default:
-			// An existing-but-unstattable backup must not be overwritten (that would
-			// destroy the recoverable original); fail before any write instead.
-			return p.fail(PhaseMerge, false, NewError(ErrMerge, statErr, "cannot stat backup %s", filepath.ToSlash(backup)))
+		// Stat+read up front: mode is preserved onto the backup, content is the
+		// no-op base and the backup source. Fail before any write (no mutation yet)
+		// rather than swallow a stat/read error on an existing pyproject.toml.
+		info, statErr := os.Stat(pyproject)
+		if statErr != nil {
+			return p.fail(PhaseMerge, false, NewError(ErrMerge, statErr, "stat pyproject.toml %s failed", filepath.ToSlash(pyproject)))
+		}
+		current, readErr := os.ReadFile(pyproject)
+		if readErr != nil {
+			return p.fail(PhaseMerge, false, NewError(ErrMerge, readErr, "read pyproject.toml %s failed", filepath.ToSlash(pyproject)))
 		}
-		p.res.BackupPath = filepath.ToSlash(backup)
 
-		// Skip the write when the merged output already matches what is on disk.
-		// On an idempotent re-run mergePlan reproduces the current file byte for
-		// byte, so rewriting it would only advance the mtime — spuriously
-		// invalidating file watchers and uv.lock freshness checks — without
-		// changing content. The backup above is untouched (the existing .bak is
-		// kept), so this leaves disk exactly as it was.
-		if current, readErr := os.ReadFile(pyproject); readErr == nil && bytes.Equal(current, mergedBytes) {
+		// No-op: the merged output already matches disk. On an idempotent re-run
+		// mergePlan reproduces the current file byte for byte, so rewriting it would
+		// only advance the mtime — spuriously invalidating file watchers and uv.lock
+		// freshness checks — without changing content. Skip both the backup and the
+		// write, leaving disk (and every existing backup) exactly as it was.
+		if bytes.Equal(current, mergedBytes) {
 			return nil
 		}
+
+		// Back up before overwriting (invariant 2). A partial backup is possible
+		// mid-write, so report disk as mutated on error.
+		backup, backupErr := p.backupCurrent(current, info.Mode().Perm())
+		if backupErr != nil {
+			return p.fail(PhaseMerge, true, NewError(ErrMerge, backupErr, "backup pyproject.toml failed"))
+		}
+		p.res.BackupPath = filepath.ToSlash(backup)
 	}
 
 	if err := os.WriteFile(pyproject, mergedBytes, 0o644); err != nil {
@@ -707,22 +765,21 @@ func sanitizeProjectName(name string) string {
 	return out
 }
 
-// copyFile copies src to dst, creating or overwriting dst. dst is created with
-// src's permission bits: the backup preserves a locked-down pyproject.toml
-// (e.g. 0o600 because it carries a private index URL) rather than widening it to
-// a hardcoded 0o644. os.WriteFile only applies the mode when it creates the
-// file, which is always the case for the freshly-created .bak.
-func copyFile(src, dst string) error {
-	info, err := os.Stat(src)
+// writeNew creates path with content, failing (os.ErrExist) rather than
+// overwriting an existing file. This no-clobber guarantee (O_EXCL) is why a
+// backup can never destroy an earlier one. mode sets the new file's permission
+// bits, so a backup keeps a locked-down pyproject.toml's permissions.
+func writeNew(path string, content []byte, mode os.FileMode) error {
+	f, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_EXCL, mode)
 	if err != nil {
-		return fmt.Errorf("stat %s: %w", src, err)
-	}
-	data, err := os.ReadFile(src)
-	if err != nil {
-		return fmt.Errorf("read %s: %w", src, err)
+		return err
 	}
-	if err := os.WriteFile(dst, data, info.Mode().Perm()); err != nil {
-		return fmt.Errorf("write %s: %w", dst, err)
+	_, werr := f.Write(content)
+	if err := errors.Join(werr, f.Close()); err != nil {
+		// Drop the partial file so it can't pose as a complete backup and, being
+		// O_EXCL-occupied, block a later run from reclaiming the name. Best-effort.
+		_ = os.Remove(path)
+		return err
 	}
 	return nil
 }