[object Object]

← back to Cli Printing Press

fix(cli): refuse publish package --dest overlay when mirror diverges from source (#1371)

1ff685373907ac9966ceacdea7628a69edf7c235 · 2026-05-13 23:22:36 -0700 · Trevin Chow

* fix(cli): refuse publish package --dest overlay when mirror diverges from source

Before stashing the existing CLI dir, walk it and refuse the overlay
if any file exists in the mirror but not in source — the failure mode
where a direct community PR against the public library would be erased
by the next publish. A new --allow-mirror-deletions flag overrides
after manual reconciliation. .manuscripts/ and build/ stay excluded;
the publish flow manages them separately.

The check is scoped to the destination category. Other categories
belong to category-migration intent, where the operator has already
accepted that the old location goes away.

Refs #1098

* fix(cli): tighten publish-package divergence-guard ergonomics from review

- Reject --allow-mirror-deletions when --dest is absent; the flag has no
  effect under --target and the previous silent accept invited
  draft-now-switch-later misuse.
- Use errors.New for the no-arg divergence message instead of
  fmt.Errorf("%s", ...).

Files touched

Diff

commit 1ff685373907ac9966ceacdea7628a69edf7c235
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Wed May 13 23:22:36 2026 -0700

    fix(cli): refuse publish package --dest overlay when mirror diverges from source (#1371)
    
    * fix(cli): refuse publish package --dest overlay when mirror diverges from source
    
    Before stashing the existing CLI dir, walk it and refuse the overlay
    if any file exists in the mirror but not in source — the failure mode
    where a direct community PR against the public library would be erased
    by the next publish. A new --allow-mirror-deletions flag overrides
    after manual reconciliation. .manuscripts/ and build/ stay excluded;
    the publish flow manages them separately.
    
    The check is scoped to the destination category. Other categories
    belong to category-migration intent, where the operator has already
    accepted that the old location goes away.
    
    Refs #1098
    
    * fix(cli): tighten publish-package divergence-guard ergonomics from review
    
    - Reject --allow-mirror-deletions when --dest is absent; the flag has no
      effect under --target and the previous silent accept invited
      draft-now-switch-later misuse.
    - Use errors.New for the no-arg divergence message instead of
      fmt.Errorf("%s", ...).
---
 internal/cli/publish.go      |  97 ++++++++++++++++++++
 internal/cli/publish_test.go | 205 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 302 insertions(+)

diff --git a/internal/cli/publish.go b/internal/cli/publish.go
index e578dd3c..cc446a8b 100644
--- a/internal/cli/publish.go
+++ b/internal/cli/publish.go
@@ -3,7 +3,9 @@ package cli
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
+	"io/fs"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -214,6 +216,7 @@ func newPublishPackageCmd() *cobra.Command {
 	var target string
 	var dest string
 	var modulePath string
+	var allowMirrorDeletions bool
 	var asJSON bool
 
 	cmd := &cobra.Command{
@@ -246,6 +249,9 @@ func newPublishPackageCmd() *cobra.Command {
 			if target != "" && dest != "" {
 				return &ExitError{Code: ExitInputError, Err: fmt.Errorf("--target and --dest are mutually exclusive")}
 			}
+			if allowMirrorDeletions && dest == "" {
+				return &ExitError{Code: ExitInputError, Err: fmt.Errorf("--allow-mirror-deletions requires --dest (the divergence guard runs only in --dest mode)")}
+			}
 
 			// Cheap existence checks before expensive validation
 			if target != "" {
@@ -297,6 +303,15 @@ func newPublishPackageCmd() *cobra.Command {
 				rootDir = dest
 				outCLIDir = filepath.Join(dest, "library", category, dirName)
 
+				// Scoped to this CLI dir; other categories belong to
+				// category-migration intent, where the operator has
+				// already accepted that the old location goes away.
+				if !allowMirrorDeletions {
+					if err := checkMirrorDivergence(outCLIDir, dir); err != nil {
+						return err
+					}
+				}
+
 				// Move existing CLI dirs aside (don't delete yet — restore on failure)
 				var err error
 				stashedDirs, err = stashExistingCLI(dest, dirName)
@@ -420,6 +435,7 @@ func newPublishPackageCmd() *cobra.Command {
 	cmd.Flags().StringVar(&target, "target", "", "Staging directory to create (mutually exclusive with --dest)")
 	cmd.Flags().StringVar(&dest, "dest", "", "Publish repo to write into directly (mutually exclusive with --target)")
 	cmd.Flags().StringVar(&modulePath, "module-path", "", "Go module path to set (e.g., github.com/org/repo/library/category/cli-name)")
+	cmd.Flags().BoolVar(&allowMirrorDeletions, "allow-mirror-deletions", false, "Allow the overlay to delete mirror files that have no source counterpart (use only after manual reconciliation)")
 	cmd.Flags().BoolVar(&asJSON, "json", false, "Output as JSON")
 
 	return cmd
@@ -477,6 +493,87 @@ func removeStashedDirs(dirs []stashedDir) {
 	}
 }
 
+// mirrorDivergenceExampleLimit caps how many would-be-deleted paths the
+// divergence error names inline before falling back to a count.
+const mirrorDivergenceExampleLimit = 10
+
+// checkMirrorDivergence returns an ExitInputError naming the first
+// findings if any file under mirrorCLIDir is not present in sourceCLIDir.
+// A non-existent mirrorCLIDir is treated as no divergence.
+func checkMirrorDivergence(mirrorCLIDir, sourceCLIDir string) error {
+	mirrorOnly, err := listMirrorOnlyFiles(mirrorCLIDir, sourceCLIDir)
+	if err != nil {
+		return &ExitError{Code: ExitPublishError, Err: fmt.Errorf("scanning mirror for divergence: %w", err)}
+	}
+	if len(mirrorOnly) == 0 {
+		return nil
+	}
+
+	var msg strings.Builder
+	noun := "files"
+	if len(mirrorOnly) == 1 {
+		noun = "file"
+	}
+	fmt.Fprintf(&msg, "mirror has %d %s not present in source library (likely a direct community PR or independent edit). Publishing now would delete this content. Reconcile manually, then re-run with --allow-mirror-deletions to override.\n", len(mirrorOnly), noun)
+	limit := min(len(mirrorOnly), mirrorDivergenceExampleLimit)
+	for _, p := range mirrorOnly[:limit] {
+		fmt.Fprintf(&msg, "  %s\n", p)
+	}
+	if len(mirrorOnly) > limit {
+		fmt.Fprintf(&msg, "  ... and %d more\n", len(mirrorOnly)-limit)
+	}
+
+	return &ExitError{Code: ExitInputError, Err: errors.New(strings.TrimRight(msg.String(), "\n"))}
+}
+
+// listMirrorOnlyFiles returns slash-separated relative paths under
+// mirrorCLIDir whose corresponding files do not exist under sourceCLIDir.
+// The top-level .manuscripts/ and build/ directories are skipped because
+// the publish flow manages those outputs separately: .manuscripts/ is
+// repopulated per run, and build/ is stripped after the source copy. A
+// non-existent mirrorCLIDir is treated as no divergence.
+func listMirrorOnlyFiles(mirrorCLIDir, sourceCLIDir string) ([]string, error) {
+	var mirrorOnly []string
+	err := filepath.WalkDir(mirrorCLIDir, func(path string, d fs.DirEntry, walkErr error) error {
+		if walkErr != nil {
+			if path == mirrorCLIDir && os.IsNotExist(walkErr) {
+				return fs.SkipAll
+			}
+			return fmt.Errorf("%s: %w", path, walkErr)
+		}
+		if path == mirrorCLIDir {
+			return nil
+		}
+
+		rel, err := filepath.Rel(mirrorCLIDir, path)
+		if err != nil {
+			return err
+		}
+
+		if d.IsDir() {
+			switch rel {
+			case ".manuscripts", "build":
+				return fs.SkipDir
+			}
+			return nil
+		}
+
+		if _, statErr := os.Lstat(filepath.Join(sourceCLIDir, rel)); statErr != nil {
+			if os.IsNotExist(statErr) {
+				mirrorOnly = append(mirrorOnly, filepath.ToSlash(rel))
+				return nil
+			}
+			return statErr
+		}
+		return nil
+	})
+	if err != nil {
+		return nil, err
+	}
+
+	return mirrorOnly, nil
+}
+
 // resolveManuscripts finds the manuscripts directory and most recent run ID
 // for a CLI. Tries API name first (SKILL convention), then CLI name (legacy
 // binary convention), then fuzzy resolve.
diff --git a/internal/cli/publish_test.go b/internal/cli/publish_test.go
index 674f91b8..98c634e2 100644
--- a/internal/cli/publish_test.go
+++ b/internal/cli/publish_test.go
@@ -2,6 +2,7 @@ package cli
 
 import (
 	"encoding/json"
+	"fmt"
 	"os"
 	"path/filepath"
 	"runtime"
@@ -1134,3 +1135,207 @@ func writePublishablePhase5Pass(t *testing.T) {
 func testSecret(parts ...string) string {
 	return strings.Join(parts, "")
 }
+
+func TestPublishPackageAllowMirrorDeletionsRequiresDest(t *testing.T) {
+	home := setLibraryTestEnv(t)
+	cliDir := filepath.Join(home, "library", "test-pp-cli")
+	writePublishableTestCLI(t, cliDir)
+
+	target := filepath.Join(t.TempDir(), "staging")
+	cmd := newPublishCmd()
+	cmd.SetArgs([]string{"package", "--dir", cliDir, "--category", "other", "--target", target, "--allow-mirror-deletions", "--json"})
+	err := cmd.Execute()
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "--allow-mirror-deletions requires --dest")
+}
+
+func TestListMirrorOnlyFiles(t *testing.T) {
+	srcDir := t.TempDir()
+	mirrorDir := t.TempDir()
+
+	// Files in both.
+	for _, p := range []string{"go.mod", "README.md", filepath.Join("internal", "cli", "root.go")} {
+		require.NoError(t, os.MkdirAll(filepath.Dir(filepath.Join(srcDir, p)), 0o755))
+		require.NoError(t, os.WriteFile(filepath.Join(srcDir, p), []byte("src"), 0o644))
+		require.NoError(t, os.MkdirAll(filepath.Dir(filepath.Join(mirrorDir, p)), 0o755))
+		require.NoError(t, os.WriteFile(filepath.Join(mirrorDir, p), []byte("mirror"), 0o644))
+	}
+
+	// Files only in mirror (the deletion risk).
+	for _, p := range []string{
+		filepath.Join("internal", "source", "resy", "client.go"),
+		filepath.Join("internal", "source", "resy", "types.go"),
+	} {
+		require.NoError(t, os.MkdirAll(filepath.Dir(filepath.Join(mirrorDir, p)), 0o755))
+		require.NoError(t, os.WriteFile(filepath.Join(mirrorDir, p), []byte("mirror-only"), 0o644))
+	}
+
+	// Files only in mirror but under excluded prefixes (should be ignored).
+	require.NoError(t, os.MkdirAll(filepath.Join(mirrorDir, ".manuscripts", "20260101-000000", "research"), 0o755))
+	require.NoError(t, os.WriteFile(filepath.Join(mirrorDir, ".manuscripts", "20260101-000000", "research", "brief.md"), []byte("old"), 0o644))
+	require.NoError(t, os.MkdirAll(filepath.Join(mirrorDir, "build"), 0o755))
+	require.NoError(t, os.WriteFile(filepath.Join(mirrorDir, "build", "host.tar.gz"), []byte("artifact"), 0o644))
+
+	// File only in source (not a deletion risk).
+	require.NoError(t, os.WriteFile(filepath.Join(srcDir, "NEW.md"), []byte("new in source"), 0o644))
+
+	got, err := listMirrorOnlyFiles(mirrorDir, srcDir)
+	require.NoError(t, err)
+	assert.Equal(t, []string{
+		"internal/source/resy/client.go",
+		"internal/source/resy/types.go",
+	}, got)
+}
+
+func TestListMirrorOnlyFilesNothingMissing(t *testing.T) {
+	srcDir := t.TempDir()
+	mirrorDir := t.TempDir()
+	for _, p := range []string{"go.mod", "README.md"} {
+		require.NoError(t, os.WriteFile(filepath.Join(srcDir, p), []byte("src"), 0o644))
+		require.NoError(t, os.WriteFile(filepath.Join(mirrorDir, p), []byte("mirror"), 0o644))
+	}
+
+	got, err := listMirrorOnlyFiles(mirrorDir, srcDir)
+	require.NoError(t, err)
+	assert.Empty(t, got)
+}
+
+func TestListMirrorOnlyFilesTopLevelFileNamedLikeExcludedDir(t *testing.T) {
+	srcDir := t.TempDir()
+	mirrorDir := t.TempDir()
+
+	// Mirror has a top-level FILE (not directory) literally named
+	// .manuscripts. The exclusion targets the .manuscripts/ directory,
+	// not files with that name, so this should be reported.
+	require.NoError(t, os.WriteFile(filepath.Join(mirrorDir, ".manuscripts"), []byte("not a dir"), 0o644))
+	require.NoError(t, os.WriteFile(filepath.Join(mirrorDir, "build"), []byte("also not a dir"), 0o644))
+
+	got, err := listMirrorOnlyFiles(mirrorDir, srcDir)
+	require.NoError(t, err)
+	assert.Equal(t, []string{".manuscripts", "build"}, got)
+}
+
+func TestPublishPackageDestDivergenceMessageTruncatesAndUsesSingular(t *testing.T) {
+	home := setLibraryTestEnv(t)
+	cliDir := filepath.Join(home, "library", "test-pp-cli")
+	writePublishableTestCLI(t, cliDir)
+
+	destDir := filepath.Join(t.TempDir(), "publish-repo")
+	mirrorCLIDir := filepath.Join(destDir, "library", "other", "test")
+	require.NoError(t, os.MkdirAll(mirrorCLIDir, 0o755))
+
+	// Singular case: exactly one mirror-only file.
+	soloDir := filepath.Join(mirrorCLIDir, "internal", "extra")
+	require.NoError(t, os.MkdirAll(soloDir, 0o755))
+	require.NoError(t, os.WriteFile(filepath.Join(soloDir, "only.go"), []byte("package extra\n"), 0o644))
+
+	cmd := newPublishCmd()
+	cmd.SetArgs([]string{"package", "--dir", cliDir, "--category", "other", "--dest", destDir, "--json"})
+	err := cmd.Execute()
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "mirror has 1 file not present")
+
+	// Truncation case: more mirror-only files than mirrorDivergenceExampleLimit.
+	overflowDir := filepath.Join(mirrorCLIDir, "internal", "overflow")
+	require.NoError(t, os.MkdirAll(overflowDir, 0o755))
+	total := mirrorDivergenceExampleLimit + 3
+	for i := range total {
+		require.NoError(t, os.WriteFile(filepath.Join(overflowDir, fmt.Sprintf("f%02d.go", i)), []byte("package overflow\n"), 0o644))
+	}
+
+	err = cmd.Execute()
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), fmt.Sprintf("mirror has %d files not present", total+1))
+	assert.Contains(t, err.Error(), fmt.Sprintf("... and %d more", total+1-mirrorDivergenceExampleLimit))
+}
+
+func TestPublishPackageDestRefusesMirrorOnlyContent(t *testing.T) {
+	home := setLibraryTestEnv(t)
+	cliDir := filepath.Join(home, "library", "test-pp-cli")
+	writePublishableTestCLI(t, cliDir)
+
+	destDir := filepath.Join(t.TempDir(), "publish-repo")
+	mirrorCLIDir := filepath.Join(destDir, "library", "other", "test")
+	require.NoError(t, os.MkdirAll(mirrorCLIDir, 0o755))
+
+	// Mirror has a package the source does not. Without the guard, the
+	// publish overlay would silently delete it.
+	resyDir := filepath.Join(mirrorCLIDir, "internal", "source", "resy")
+	require.NoError(t, os.MkdirAll(resyDir, 0o755))
+	require.NoError(t, os.WriteFile(filepath.Join(resyDir, "client.go"), []byte("package resy\n"), 0o644))
+	require.NoError(t, os.WriteFile(filepath.Join(resyDir, "types.go"), []byte("package resy\n"), 0o644))
+
+	cmd := newPublishCmd()
+	cmd.SetArgs([]string{"package", "--dir", cliDir, "--category", "other", "--dest", destDir, "--json"})
+
+	err := cmd.Execute()
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "mirror has 2 files not present in source library")
+	assert.Contains(t, err.Error(), "internal/source/resy/client.go")
+	assert.Contains(t, err.Error(), "internal/source/resy/types.go")
+	assert.Contains(t, err.Error(), "--allow-mirror-deletions")
+
+	// Mirror content must remain intact after refusal.
+	_, err = os.Stat(filepath.Join(resyDir, "client.go"))
+	assert.NoError(t, err, "mirror content should remain after refusal")
+	_, err = os.Stat(mirrorCLIDir + ".old")
+	assert.ErrorIs(t, err, os.ErrNotExist, "should not have stashed before failing")
+}
+
+func TestPublishPackageDestAllowMirrorDeletionsOverride(t *testing.T) {
+	home := setLibraryTestEnv(t)
+	cliDir := filepath.Join(home, "library", "test-pp-cli")
+	writePublishableTestCLI(t, cliDir)
+
+	destDir := filepath.Join(t.TempDir(), "publish-repo")
+	mirrorCLIDir := filepath.Join(destDir, "library", "other", "test")
+	resyDir := filepath.Join(mirrorCLIDir, "internal", "source", "resy")
+	require.NoError(t, os.MkdirAll(resyDir, 0o755))
+	require.NoError(t, os.WriteFile(filepath.Join(resyDir, "client.go"), []byte("package resy\n"), 0o644))
+
+	cmd := newPublishCmd()
+	cmd.SetArgs([]string{"package", "--dir", cliDir, "--category", "other", "--dest", destDir, "--allow-mirror-deletions", "--json"})
+
+	output, err := runWithCapturedStdout(t, cmd.Execute)
+	require.NoError(t, err)
+	var result PackageResult
+	require.NoError(t, json.Unmarshal([]byte(output), &result))
+	assert.Equal(t, filepath.Join(destDir, "library", "other", "test"), result.StagedDir)
+
+	// Mirror-only file is gone after the override.
+	_, err = os.Stat(filepath.Join(resyDir, "client.go"))
+	assert.ErrorIs(t, err, os.ErrNotExist, "mirror-only file should be deleted with override")
+}
+
+func TestPublishPackageDestIgnoresManuscriptsDivergence(t *testing.T) {
+	home := setLibraryTestEnv(t)
+	cliDir := filepath.Join(home, "library", "test-pp-cli")
+	writePublishableTestCLI(t, cliDir)
+
+	// Create manuscripts for the new run.
+	runID := "20260329-100000"
+	researchDir := filepath.Join(home, "manuscripts", "test", runID, "research")
+	require.NoError(t, os.MkdirAll(researchDir, 0o755))
+	require.NoError(t, os.WriteFile(filepath.Join(researchDir, "brief.md"), []byte("# Brief"), 0o644))
+
+	destDir := filepath.Join(t.TempDir(), "publish-repo")
+	mirrorCLIDir := filepath.Join(destDir, "library", "other", "test")
+
+	// Mirror has an old manuscripts run and a stale build/ dir. Neither
+	// represents real divergence; both are managed by the publish flow.
+	oldMSDir := filepath.Join(mirrorCLIDir, ".manuscripts", "20260101-000000", "research")
+	require.NoError(t, os.MkdirAll(oldMSDir, 0o755))
+	require.NoError(t, os.WriteFile(filepath.Join(oldMSDir, "brief.md"), []byte("old"), 0o644))
+	require.NoError(t, os.MkdirAll(filepath.Join(mirrorCLIDir, "build"), 0o755))
+	require.NoError(t, os.WriteFile(filepath.Join(mirrorCLIDir, "build", "host.tar.gz"), []byte("artifact"), 0o644))
+
+	cmd := newPublishCmd()
+	cmd.SetArgs([]string{"package", "--dir", cliDir, "--category", "other", "--dest", destDir, "--json"})
+
+	output, err := runWithCapturedStdout(t, cmd.Execute)
+	require.NoError(t, err, "manuscripts/build divergence should not block the overlay")
+
+	var result PackageResult
+	require.NoError(t, json.Unmarshal([]byte(output), &result))
+	assert.True(t, result.ManuscriptsIncluded)
+}

← 2437cab7 fix(cli): wire composed apiKey + bearer sibling headers thro  ·  back to Cli Printing Press  ·  chore(main): release 4.6.1 (#1363) 725e063f →