[object Object]

← back to Cli Printing Press

fix(cli): strip root-level binaries from staged publish tree (#1449)

096411cea4838d7c9dc27be4acfbc5bd4627a076 · 2026-05-15 10:58:23 -0700 · Trevin Chow

* fix(cli): strip root-level binaries from staged publish tree

The publish skill's rm -f cleanup line listed two binary names but
missed <api-slug>-pp-mcp, so every CLI with MCP vision shipped a 21MB
Mach-O peer into the publish PR unless the operator caught it by hand.

Strip the named root-level binaries (bare slug, <api-slug>-pp-cli,
<cli-name>, <api-slug>-pp-mcp) in publish package after CopyDir, so
the staged tree is binary-free regardless of which path produced them
(Makefile build / build-mcp / hand-run go build ./cmd/...). The SKILL
rm -f line gets the pp-mcp entry too for the agent path.

Closes #1396

* docs(cli): clarify Greptile-flagged comments in publish-strip path

Greptile pointed out two minor ambiguities:

- `internal/cli/publish.go:364`: the phrase "before the copy" read as
  "before pipeline.CopyDir" given the adjacent call, when the intended
  reference was the skill's downstream `cp -r` step. Reword to name the
  skill's `rm -f` parallel and the staged-copy timing explicitly.

- `internal/cli/publish_test.go:582`: the inline comment said the skill
  cleanup "currently misses" the pp-mcp binary, but this PR's SKILL.md
  edit fixed that. Update to state both layers now cover it.

No code changes. Refs #1396.

Files touched

Diff

commit 096411cea4838d7c9dc27be4acfbc5bd4627a076
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Fri May 15 10:58:23 2026 -0700

    fix(cli): strip root-level binaries from staged publish tree (#1449)
    
    * fix(cli): strip root-level binaries from staged publish tree
    
    The publish skill's rm -f cleanup line listed two binary names but
    missed <api-slug>-pp-mcp, so every CLI with MCP vision shipped a 21MB
    Mach-O peer into the publish PR unless the operator caught it by hand.
    
    Strip the named root-level binaries (bare slug, <api-slug>-pp-cli,
    <cli-name>, <api-slug>-pp-mcp) in publish package after CopyDir, so
    the staged tree is binary-free regardless of which path produced them
    (Makefile build / build-mcp / hand-run go build ./cmd/...). The SKILL
    rm -f line gets the pp-mcp entry too for the agent path.
    
    Closes #1396
    
    * docs(cli): clarify Greptile-flagged comments in publish-strip path
    
    Greptile pointed out two minor ambiguities:
    
    - `internal/cli/publish.go:364`: the phrase "before the copy" read as
      "before pipeline.CopyDir" given the adjacent call, when the intended
      reference was the skill's downstream `cp -r` step. Reword to name the
      skill's `rm -f` parallel and the staged-copy timing explicitly.
    
    - `internal/cli/publish_test.go:582`: the inline comment said the skill
      cleanup "currently misses" the pp-mcp binary, but this PR's SKILL.md
      edit fixed that. Update to state both layers now cover it.
    
    No code changes. Refs #1396.
---
 internal/cli/publish.go                | 43 +++++++++++++++++++++++++
 internal/cli/publish_test.go           | 57 ++++++++++++++++++++++++++++++++++
 skills/printing-press-publish/SKILL.md |  9 ++++--
 3 files changed, 107 insertions(+), 2 deletions(-)

diff --git a/internal/cli/publish.go b/internal/cli/publish.go
index cc446a8b..1e2dfe8e 100644
--- a/internal/cli/publish.go
+++ b/internal/cli/publish.go
@@ -361,6 +361,20 @@ func newPublishPackageCmd() *cobra.Command {
 				return &ExitError{Code: ExitPublishError, Err: fmt.Errorf("stripping build dir: %w", err)}
 			}
 
+			// Strip root-level binaries that local `go build ./cmd/...` (or
+			// `make build` / `make build-mcp` without `-o bin/...`) drops at
+			// the CLI dir root. The skill's downstream `cp -r ... PUBLISH_REPO_DIR`
+			// has a parallel `rm -f` line that historically missed
+			// `<api-slug>-pp-mcp`; doing the strip here on the staged copy
+			// makes the publish path binary-free regardless of which downstream
+			// step (skill or other tooling) the operator runs next.
+			for _, name := range stagedBinaryNames(cliName, dirName) {
+				if err := os.Remove(filepath.Join(outCLIDir, name)); err != nil && !os.IsNotExist(err) {
+					cleanupOnFailure()
+					return &ExitError{Code: ExitPublishError, Err: fmt.Errorf("stripping staged binary %s: %w", name, err)}
+				}
+			}
+
 			// Rewrite go.mod module path if --module-path is set
 			if modulePath != "" {
 				oldModPath := cliName // generated CLIs use bare CLI name as module path
@@ -994,6 +1008,35 @@ func buildArtifactCandidates(dir, cliName string) []string {
 	}
 }
 
+// stagedBinaryNames returns the root-level filenames that local builds
+// (`go build ./cmd/...`, `make build`, `make build-mcp` without `-o`)
+// drop alongside the source tree. The Makefile template emits
+// `<api-slug>-pp-cli` and `<api-slug>-pp-mcp` peers, and verifier paths
+// may also leave a bare `<api-slug>` artifact. The list is intentionally
+// a small, named superset rather than a glob so the strip step never
+// removes a tracked source file by accident.
+func stagedBinaryNames(cliName, apiSlug string) []string {
+	seen := map[string]struct{}{}
+	var names []string
+	add := func(n string) {
+		if n == "" {
+			return
+		}
+		if _, ok := seen[n]; ok {
+			return
+		}
+		seen[n] = struct{}{}
+		names = append(names, n)
+	}
+	add(apiSlug)
+	add(cliName)
+	if apiSlug != "" {
+		add(apiSlug + "-pp-cli")
+		add(apiSlug + "-pp-mcp")
+	}
+	return names
+}
+
 type fileSnapshot struct {
 	path    string
 	exists  bool
diff --git a/internal/cli/publish_test.go b/internal/cli/publish_test.go
index 98c634e2..e0fc5dcd 100644
--- a/internal/cli/publish_test.go
+++ b/internal/cli/publish_test.go
@@ -566,6 +566,63 @@ func TestPublishPackageStripsBuildDir(t *testing.T) {
 	assert.ErrorIs(t, stagedErr, os.ErrNotExist, "staged dir must not include build/")
 }
 
+func TestPublishPackageStripsRootBinaries(t *testing.T) {
+	home := setLibraryTestEnv(t)
+	cliDir := filepath.Join(home, "library", "test-pp-cli")
+	writePublishableTestCLI(t, cliDir)
+
+	// Simulate an operator who ran `go build ./cmd/...` (or `make build` /
+	// `make build-mcp` without `-o bin/...`): binaries land at the CLI
+	// dir root, named per the generator convention. None of these should
+	// reach the staged tree, regardless of which one the publish skill's
+	// rm -f line happened to enumerate.
+	for _, name := range []string{
+		"test",        // phantom bare-slug binary
+		"test-pp-cli", // primary CLI binary
+		"test-pp-mcp", // MCP peer binary (now covered by both code-level strip and skill cleanup)
+	} {
+		require.NoError(t, os.WriteFile(filepath.Join(cliDir, name), []byte("\x7fELF"), 0o755))
+	}
+
+	target := filepath.Join(t.TempDir(), "staging")
+	cmd := newPublishCmd()
+	cmd.SetArgs([]string{"package", "--dir", cliDir, "--category", "other", "--target", target, "--json"})
+
+	output, err := runWithCapturedStdout(t, cmd.Execute)
+	require.NoError(t, err)
+
+	var result PackageResult
+	require.NoError(t, json.Unmarshal([]byte(output), &result))
+
+	// Source binaries stay — we don't mutate the operator's working tree.
+	for _, name := range []string{"test", "test-pp-cli", "test-pp-mcp"} {
+		_, srcErr := os.Stat(filepath.Join(cliDir, name))
+		assert.NoError(t, srcErr, "package must not delete source-tree binary %s", name)
+	}
+
+	// Staged tree must have none of them.
+	for _, name := range []string{"test", "test-pp-cli", "test-pp-mcp"} {
+		_, stagedErr := os.Stat(filepath.Join(result.StagedDir, name))
+		assert.ErrorIs(t, stagedErr, os.ErrNotExist, "staged dir must not include root-level binary %s", name)
+	}
+}
+
+func TestStagedBinaryNamesDeduplicates(t *testing.T) {
+	t.Parallel()
+
+	got := stagedBinaryNames("foo-pp-cli", "foo")
+	want := []string{"foo", "foo-pp-cli", "foo-pp-mcp"}
+	assert.Equal(t, want, got)
+
+	// Empty slug returns only the cliName (no -pp-cli/-pp-mcp suffix expansion).
+	got = stagedBinaryNames("custom-binary", "")
+	want = []string{"custom-binary"}
+	assert.Equal(t, want, got)
+
+	// Empty both is empty.
+	assert.Empty(t, stagedBinaryNames("", ""))
+}
+
 func TestPublishPackageFailsWhenManuscriptsCopyFails(t *testing.T) {
 	skipIfRootCannotSimulateUnreadable(t)
 	home := setLibraryTestEnv(t)
diff --git a/skills/printing-press-publish/SKILL.md b/skills/printing-press-publish/SKILL.md
index 17814752..4f599d47 100644
--- a/skills/printing-press-publish/SKILL.md
+++ b/skills/printing-press-publish/SKILL.md
@@ -436,8 +436,13 @@ rm -rf "$PUBLISH_REPO_DIR/library"/*/"<api-slug>"
 # Copy staged CLI into publish repo (slug-keyed directory)
 cp -r "$STAGING_DIR/library/<category>/<api-slug>" "$PUBLISH_REPO_DIR/library/<category>/<api-slug>"
 
-# Remove binaries (should not be committed)
-rm -f "$PUBLISH_REPO_DIR/library/<category>/<api-slug>/<api-slug>" "$PUBLISH_REPO_DIR/library/<category>/<api-slug>/<cli-name>"
+# Remove root-level binaries (should not be committed). publish package
+# already strips these before the copy; this rm -f is belt-and-suspenders
+# for the agent path. Cover all three names the Makefile/`go build ./cmd/...`
+# can drop: bare slug, CLI binary, MCP peer.
+rm -f "$PUBLISH_REPO_DIR/library/<category>/<api-slug>/<api-slug>" \
+      "$PUBLISH_REPO_DIR/library/<category>/<api-slug>/<cli-name>" \
+      "$PUBLISH_REPO_DIR/library/<category>/<api-slug>/<api-slug>-pp-mcp"
 
 # Defense-in-depth: validate printer attribution before README and registry surfaces.
 PRINTER=$(jq -r '.printer // ""' "$PUBLISH_REPO_DIR/library/<category>/<api-slug>/.printing-press.json")

← b8b1b6a4 fix(cli): force sync concurrency=1 under PRINTING_PRESS_VERI  ·  back to Cli Printing Press  ·  fix(skills): correct install-internal-skills.sh path to repo 5a04aca6 →