← back to Cli Printing Press
fix(cli): mcp-sync detects suggestFlag/Deliver across cli package, not just root.go (#377)
a81630244c58a87f8fc3fc3693267f39dd0581a5 · 2026-04-28 22:21:21 -0700 · Trevin Chow
* fix(cli): mcp-sync detects suggestFlag/Deliver across cli package, not just root.go
#376 added conditional inclusion of suggestFlag and Deliver prolog blocks
based on string-scanning root.go for `func suggestFlag(` and `func
Deliver(`. But by generator convention those functions live in sibling
files (helpers.go and deliver.go respectively), not in root.go itself.
So scoping detection to root.go misfired in the opposite direction:
real CLIs that DO have the features got empty prolog blocks because
the function definitions lived next door.
Surfaced when backfilling the deliver feature into scrape-creators
(library/developer-tools/scrape-creators) — copying deliver.go into
the package and adding the rootFlags fields wasn't enough; mcp-sync
still emitted the bare prolog because it only looked at root.go.
New helper `pkgContainsDecl(pkgDir, decl)` reads every .go file in
the cli package and reports whether decl appears in any of them.
Skips _test.go files so test fixtures don't trigger false positives.
Returns (false, nil) when the package directory is missing — keeps
the existing error path intact for genuinely broken layouts.
Test: TestEnsureRootCmdExportDetectsFeaturesAcrossPackage seeds a
package with Deliver in deliver.go and suggestFlag in helpers.go
(real generator layout), asserts the prolog includes both blocks.
Tests: 2233/2233. Golden 9/9. Lint clean.
* simplify: trim narrating comments per /simplify pass
Three trims, all comments. No code changes.
- pkgContainsDecl godoc: drop the call-site narration ("Used to
detect opt-in template features (suggestFlag, Deliver) whose
supporting code lives in sibling files rather than root.go
itself."). Godoc shouldn't bind to a single caller; the WHY belongs
with the call site, not the helper.
- ensureRootCmdExport call-site comment: remove the redundant
"Scan the whole cli package, not just root.go: by convention the
generator emits suggestFlag in helpers.go and Deliver in
deliver.go." The function name plus pkgDir line already say "scan
the package," and the WHY (older CLIs predate features) is in the
comment immediately above.
- Test docstring: drop the trailing bug-narration ("By generator
convention they always live in siblings, so detection scoped to
root.go alone would miss every real CLI and emit empty prolog
blocks."). Test name + first sentence already state the contract.
Tests: 19/19 pass in mcpsync package.
Files touched
M internal/pipeline/mcpsync/sync.goM internal/pipeline/mcpsync/sync_test.go
Diff
commit a81630244c58a87f8fc3fc3693267f39dd0581a5
Author: Trevin Chow <trevin@trevinchow.com>
Date: Tue Apr 28 22:21:21 2026 -0700
fix(cli): mcp-sync detects suggestFlag/Deliver across cli package, not just root.go (#377)
* fix(cli): mcp-sync detects suggestFlag/Deliver across cli package, not just root.go
#376 added conditional inclusion of suggestFlag and Deliver prolog blocks
based on string-scanning root.go for `func suggestFlag(` and `func
Deliver(`. But by generator convention those functions live in sibling
files (helpers.go and deliver.go respectively), not in root.go itself.
So scoping detection to root.go misfired in the opposite direction:
real CLIs that DO have the features got empty prolog blocks because
the function definitions lived next door.
Surfaced when backfilling the deliver feature into scrape-creators
(library/developer-tools/scrape-creators) — copying deliver.go into
the package and adding the rootFlags fields wasn't enough; mcp-sync
still emitted the bare prolog because it only looked at root.go.
New helper `pkgContainsDecl(pkgDir, decl)` reads every .go file in
the cli package and reports whether decl appears in any of them.
Skips _test.go files so test fixtures don't trigger false positives.
Returns (false, nil) when the package directory is missing — keeps
the existing error path intact for genuinely broken layouts.
Test: TestEnsureRootCmdExportDetectsFeaturesAcrossPackage seeds a
package with Deliver in deliver.go and suggestFlag in helpers.go
(real generator layout), asserts the prolog includes both blocks.
Tests: 2233/2233. Golden 9/9. Lint clean.
* simplify: trim narrating comments per /simplify pass
Three trims, all comments. No code changes.
- pkgContainsDecl godoc: drop the call-site narration ("Used to
detect opt-in template features (suggestFlag, Deliver) whose
supporting code lives in sibling files rather than root.go
itself."). Godoc shouldn't bind to a single caller; the WHY belongs
with the call site, not the helper.
- ensureRootCmdExport call-site comment: remove the redundant
"Scan the whole cli package, not just root.go: by convention the
generator emits suggestFlag in helpers.go and Deliver in
deliver.go." The function name plus pkgDir line already say "scan
the package," and the WHY (older CLIs predate features) is in the
comment immediately above.
- Test docstring: drop the trailing bug-narration ("By generator
convention they always live in siblings, so detection scoped to
root.go alone would miss every real CLI and emit empty prolog
blocks."). Test name + first sentence already state the contract.
Tests: 19/19 pass in mcpsync package.
---
internal/pipeline/mcpsync/sync.go | 38 ++++++++++++++++++-
internal/pipeline/mcpsync/sync_test.go | 67 ++++++++++++++++++++++++++++++++++
2 files changed, 103 insertions(+), 2 deletions(-)
diff --git a/internal/pipeline/mcpsync/sync.go b/internal/pipeline/mcpsync/sync.go
index 0bae3b16..fe8baf8d 100644
--- a/internal/pipeline/mcpsync/sync.go
+++ b/internal/pipeline/mcpsync/sync.go
@@ -272,6 +272,32 @@ func ensureEndpointAnnotation(path, annotationLine string) error {
return writeFileAtomic(path, []byte(next))
}
+// pkgContainsDecl reports whether any .go file in pkgDir contains decl.
+// Skips _test.go files so test fixtures don't trigger false positives.
+func pkgContainsDecl(pkgDir, decl string) (bool, error) {
+ entries, err := os.ReadDir(pkgDir)
+ if err != nil {
+ if errors.Is(err, os.ErrNotExist) {
+ return false, nil
+ }
+ return false, err
+ }
+ for _, e := range entries {
+ name := e.Name()
+ if e.IsDir() || !strings.HasSuffix(name, ".go") || strings.HasSuffix(name, "_test.go") {
+ continue
+ }
+ data, err := os.ReadFile(filepath.Join(pkgDir, name))
+ if err != nil {
+ return false, err
+ }
+ if strings.Contains(string(data), decl) {
+ return true, nil
+ }
+ }
+ return false, nil
+}
+
func ensureRootCmdExport(cliDir string) error {
path := filepath.Join(cliDir, "internal", "cli", "root.go")
data, err := os.ReadFile(path)
@@ -298,8 +324,16 @@ func ensureRootCmdExport(cliDir string) error {
// 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")
+ pkgDir := filepath.Join(cliDir, "internal", "cli")
+ hasSuggestFlag, err := pkgContainsDecl(pkgDir, "func suggestFlag(")
+ if err != nil {
+ return fmt.Errorf("scanning cli package for suggestFlag: %w", err)
+ }
+ hasDeliverFunc, err := pkgContainsDecl(pkgDir, "func Deliver(")
+ if err != nil {
+ return fmt.Errorf("scanning cli package for Deliver: %w", err)
+ }
+ hasDeliver := hasDeliverFunc && strings.Contains(src, "deliverBuf")
suggestBlock := ""
if hasSuggestFlag {
diff --git a/internal/pipeline/mcpsync/sync_test.go b/internal/pipeline/mcpsync/sync_test.go
index b3d6e763..5160ebab 100644
--- a/internal/pipeline/mcpsync/sync_test.go
+++ b/internal/pipeline/mcpsync/sync_test.go
@@ -567,6 +567,73 @@ func ExitCode(err error) int { return 0 }
}
}
+// TestEnsureRootCmdExportDetectsFeaturesAcrossPackage asserts that
+// suggestFlag and Deliver are detected even when their definitions
+// live in sibling files (helpers.go, deliver.go) rather than root.go.
+func TestEnsureRootCmdExportDetectsFeaturesAcrossPackage(t *testing.T) {
+ t.Parallel()
+
+ cliDir := t.TempDir()
+ pkgDir := filepath.Join(cliDir, "internal", "cli")
+ require.NoError(t, os.MkdirAll(pkgDir, 0o755))
+
+ // root.go has the deliverBuf field (so the deliver block applies)
+ // but neither Deliver nor suggestFlag are defined here.
+ require.NoError(t, os.WriteFile(filepath.Join(pkgDir, "root.go"), []byte(`// 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
+}
+
+func Execute() error {
+ var flags rootFlags
+
+ rootCmd := &cobra.Command{
+ Use: "splittest-pp-cli",
+ }
+ rootCmd.PersistentFlags().BoolVar(&flags.asJSON, "json", false, "")
+ err := rootCmd.Execute()
+ return err
+}
+
+func ExitCode(err error) int { return 0 }
+`), 0o644))
+
+ // deliver.go provides the Deliver function (real generator layout).
+ require.NoError(t, os.WriteFile(filepath.Join(pkgDir, "deliver.go"), []byte(`package cli
+
+type DeliverSink struct{ Scheme, Target string }
+
+func Deliver(s DeliverSink, b []byte, c bool) error { return nil }
+`), 0o644))
+
+ // helpers.go provides suggestFlag (real generator layout).
+ require.NoError(t, os.WriteFile(filepath.Join(pkgDir, "helpers.go"), []byte(`package cli
+
+import "github.com/spf13/cobra"
+
+func suggestFlag(name string, cmd *cobra.Command) string { return "" }
+`), 0o644))
+
+ require.NoError(t, ensureRootCmdExport(cliDir))
+
+ updated, err := os.ReadFile(filepath.Join(pkgDir, "root.go"))
+ require.NoError(t, err)
+ src := string(updated)
+
+ assert.Contains(t, src, "suggestFlag(", "suggestFlag detected in helpers.go must enable the suggestFlag block")
+ assert.Contains(t, src, "Deliver(flags.deliverSink", "Deliver detected in deliver.go must enable the deliver block")
+}
+
// 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.
← 2b9f2fbb fix(cli): mcp-sync skips deliver/suggestFlag prolog blocks w
·
back to Cli Printing Press
·
feat(cli): add tools-audit subcommand and merge polish into e4a0089c →