[object Object]

← back to Cli Printing Press

fix(cli): mcp-sync skips deliver/suggestFlag prolog blocks when absent (#376)

2b9f2fbbecac2c8a2c4a622cd4966f0004d8fab7 · 2026-04-28 21:57:12 -0700 · Trevin Chow

ensureRootCmdExport unconditionally inserted suggestFlag and Deliver
blocks into the regenerated Execute body. Older library CLIs that
predate either feature lack the supporting types/functions, so the
inserted code failed to build with "undefined: suggestFlag",
"undefined: Deliver", and "type rootFlags has no field deliverBuf".

Detect each feature by scanning the existing source for its definition
site (`func suggestFlag(`, `func Deliver(` AND `deliverBuf`) and
include the corresponding block only when the supporting code is
present. Modern CLIs continue to get both blocks; old CLIs get the
RootCmd export without phantom dependencies.

Surfaced when rebasing the scrape-creators PR (#113 in the library
repo) onto current main: scrape-creators predates both features and
mcp-sync's regen produced a non-compiling root.go. Same pattern will
hit every old CLI in the upcoming bulk sweep.

Test: TestEnsureRootCmdExportConditionalBlocks (table-driven, two
cases — old template lacks features, modern template has both) pins
the contract.

Files touched

Diff

commit 2b9f2fbbecac2c8a2c4a622cd4966f0004d8fab7
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Tue Apr 28 21:57:12 2026 -0700

    fix(cli): mcp-sync skips deliver/suggestFlag prolog blocks when absent (#376)
    
    ensureRootCmdExport unconditionally inserted suggestFlag and Deliver
    blocks into the regenerated Execute body. Older library CLIs that
    predate either feature lack the supporting types/functions, so the
    inserted code failed to build with "undefined: suggestFlag",
    "undefined: Deliver", and "type rootFlags has no field deliverBuf".
    
    Detect each feature by scanning the existing source for its definition
    site (`func suggestFlag(`, `func Deliver(` AND `deliverBuf`) and
    include the corresponding block only when the supporting code is
    present. Modern CLIs continue to get both blocks; old CLIs get the
    RootCmd export without phantom dependencies.
    
    Surfaced when rebasing the scrape-creators PR (#113 in the library
    repo) onto current main: scrape-creators predates both features and
    mcp-sync's regen produced a non-compiling root.go. Same pattern will
    hit every old CLI in the upcoming bulk sweep.
    
    Test: TestEnsureRootCmdExportConditionalBlocks (table-driven, two
    cases — old template lacks features, modern template has both) pins
    the contract.
---
 internal/pipeline/mcpsync/sync.go      |  39 +++++++++----
 internal/pipeline/mcpsync/sync_test.go | 104 +++++++++++++++++++++++++++++++++
 2 files changed, 131 insertions(+), 12 deletions(-)

diff --git a/internal/pipeline/mcpsync/sync.go b/internal/pipeline/mcpsync/sync.go
index a3e9c15b..0bae3b16 100644
--- a/internal/pipeline/mcpsync/sync.go
+++ b/internal/pipeline/mcpsync/sync.go
@@ -295,19 +295,15 @@ func ensureRootCmdExport(cliDir string) error {
 	if start < 0 {
 		return fmt.Errorf("root.go does not match the generated Execute shape; cannot add RootCmd export automatically")
 	}
-	prolog := `// RootCmd returns the Cobra command tree without executing it. The MCP server
-// uses this to mirror every user-facing command as an agent tool.
-func RootCmd() *cobra.Command {
-	var flags rootFlags
-	return newRootCmd(&flags)
-}
+	// Skip prolog blocks whose supporting types/functions aren't in the
+	// existing source — older library CLIs predate suggestFlag and
+	// Deliver and would otherwise fail to build with "undefined" errors.
+	hasSuggestFlag := strings.Contains(src, "func suggestFlag(")
+	hasDeliver := strings.Contains(src, "func Deliver(") && strings.Contains(src, "deliverBuf")
 
-// Execute runs the CLI in non-interactive mode: never prompts, all values via flags or stdin.
-func Execute() error {
-	var flags rootFlags
-	rootCmd := newRootCmd(&flags)
-
-	err := rootCmd.Execute()
+	suggestBlock := ""
+	if hasSuggestFlag {
+		suggestBlock = `
 	if err != nil && strings.Contains(err.Error(), "unknown flag") {
 		msg := err.Error()
 		// Extract the flag name from the error message (e.g., "unknown flag: --foob")
@@ -317,13 +313,32 @@ func Execute() error {
 				return fmt.Errorf("%w\nhint: did you mean --%s?", err, suggestion)
 			}
 		}
+	}`
 	}
+	deliverBlock := ""
+	if hasDeliver {
+		deliverBlock = `
 	if err == nil && flags.deliverBuf != nil {
 		if derr := Deliver(flags.deliverSink, flags.deliverBuf.Bytes(), flags.compact); derr != nil {
 			fmt.Fprintf(os.Stderr, "warning: deliver to %s:%s failed: %v\n", flags.deliverSink.Scheme, flags.deliverSink.Target, derr)
 			return derr
 		}
+	}`
 	}
+
+	prolog := `// RootCmd returns the Cobra command tree without executing it. The MCP server
+// uses this to mirror every user-facing command as an agent tool.
+func RootCmd() *cobra.Command {
+	var flags rootFlags
+	return newRootCmd(&flags)
+}
+
+// Execute runs the CLI in non-interactive mode: never prompts, all values via flags or stdin.
+func Execute() error {
+	var flags rootFlags
+	rootCmd := newRootCmd(&flags)
+
+	err := rootCmd.Execute()` + suggestBlock + deliverBlock + `
 	return err
 }
 
diff --git a/internal/pipeline/mcpsync/sync_test.go b/internal/pipeline/mcpsync/sync_test.go
index 0c329700..b3d6e763 100644
--- a/internal/pipeline/mcpsync/sync_test.go
+++ b/internal/pipeline/mcpsync/sync_test.go
@@ -463,6 +463,110 @@ func ExitCode(err error) int { return 0 }
 	assert.Contains(t, src, "return newRootCmd(&flags)")
 }
 
+// TestEnsureRootCmdExportConditionalBlocks asserts the prolog includes
+// suggestFlag and Deliver blocks only when their supporting definitions
+// exist in the source. Without conditional inclusion, older library
+// CLIs that predate both features fail to build with "undefined" errors.
+func TestEnsureRootCmdExportConditionalBlocks(t *testing.T) {
+	t.Parallel()
+
+	const oldTemplate = `// Generated by CLI Printing Press (https://github.com/mvanhorn/cli-printing-press). DO NOT EDIT.
+package cli
+
+import "github.com/spf13/cobra"
+
+type rootFlags struct{ asJSON bool }
+
+func Execute() error {
+	var flags rootFlags
+
+	rootCmd := &cobra.Command{
+		Use: "oldtemplate-pp-cli",
+	}
+	rootCmd.PersistentFlags().BoolVar(&flags.asJSON, "json", false, "")
+	err := rootCmd.Execute()
+	return err
+}
+
+func ExitCode(err error) int { return 0 }
+`
+
+	const modernTemplate = `// Generated by CLI Printing Press (https://github.com/mvanhorn/cli-printing-press). DO NOT EDIT.
+package cli
+
+import (
+	"bytes"
+	"github.com/spf13/cobra"
+)
+
+type rootFlags struct {
+	asJSON      bool
+	compact     bool
+	deliverBuf  *bytes.Buffer
+	deliverSink DeliverSink
+}
+
+type DeliverSink struct{ Scheme, Target string }
+
+func Deliver(s DeliverSink, b []byte, c bool) error { return nil }
+
+func suggestFlag(name string, cmd *cobra.Command) string { return "" }
+
+func Execute() error {
+	var flags rootFlags
+
+	rootCmd := &cobra.Command{
+		Use: "modern-pp-cli",
+	}
+	rootCmd.PersistentFlags().BoolVar(&flags.asJSON, "json", false, "")
+	err := rootCmd.Execute()
+	return err
+}
+
+func ExitCode(err error) int { return 0 }
+`
+
+	cases := []struct {
+		name        string
+		fixture     string
+		wantSuggest bool
+		wantDeliver bool
+	}{
+		{"old template lacks both features", oldTemplate, false, false},
+		{"modern template has both features", modernTemplate, true, true},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			cliDir := t.TempDir()
+			require.NoError(t, os.MkdirAll(filepath.Join(cliDir, "internal", "cli"), 0o755))
+			rootPath := filepath.Join(cliDir, "internal", "cli", "root.go")
+			require.NoError(t, os.WriteFile(rootPath, []byte(tc.fixture), 0o644))
+
+			require.NoError(t, ensureRootCmdExport(cliDir))
+
+			updated, err := os.ReadFile(rootPath)
+			require.NoError(t, err)
+			src := string(updated)
+
+			// Canonical RootCmd export always emitted.
+			assert.Contains(t, src, "func RootCmd() *cobra.Command")
+			assert.Contains(t, src, "return newRootCmd(&flags)")
+
+			if tc.wantSuggest {
+				assert.Contains(t, src, "suggestFlag(")
+			} else {
+				assert.NotContains(t, src, "suggestFlag(")
+			}
+			if tc.wantDeliver {
+				assert.Contains(t, src, "Deliver(flags.deliverSink")
+			} else {
+				assert.NotContains(t, src, "Deliver(flags.deliverSink")
+				assert.NotContains(t, src, "flags.deliverBuf")
+			}
+		})
+	}
+}
+
 // TestEnsureRootCmdExportIsIdempotent locks the no-op behavior on a root.go
 // that already has RootCmd. Future template changes that add or remove
 // surrounding code must not reintroduce the export and double-define it.

← 40d1ffad fix(cli): mcp-sync refreshes .printing-press.json from spec.  ·  back to Cli Printing Press  ·  fix(cli): mcp-sync detects suggestFlag/Deliver across cli pa a8163024 →