From 85dd5dc2fedb83d9cc39c2663f057e278b9ef35d Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Wed, 28 Oct 2026 08:39:01 -0400 Subject: [PATCH 1/8] fix(canon): suppress identity breakout deprecation warnings (D-1116) --- .github/wiki/Command-Canon.md | 2 ++ cmd/commands/canon_engine_argv.go | 4 ++++ cmd/commands/canon_engine_argv_test.go | 24 ++++++++++++++++++++++++ 3 files changed, 30 insertions(+) diff --git a/.github/wiki/Command-Canon.md b/.github/wiki/Command-Canon.md index 934b2cf66..18d004777 100644 --- a/.github/wiki/Command-Canon.md +++ b/.github/wiki/Command-Canon.md @@ -76,6 +76,8 @@ Total rows: 60 ## Removal plan +Identity break-outs (`ci` to `ci`, `functions` to `functions`) keep their spelling and do not emit a deprecation warning, as an exception to the old-spelling warnings below. Longer moves beneath them, such as `ci eval` to `eval`, still warn. + v1.5.0 ships this canon. Every old spelling above keeps working through all v1.5.x releases and prints a deprecation warning. **v1.6.0 removes every move, shim and retired hub in the table**: delete the rows from the canon fragments (`internal/canon/domains/`), run `make canon cmd-inventory wiki-commands parity`, and the old spellings are gone. `TestCanonShimLifetime` fails the build once the running version reaches a row's `removal_at`, so no shim can outlive v1.6.0 by accident. Break-outs and removals have no removal date: the command leaves core at v1.5.0. `scripts/ci/old-spellings.sh` prints every Go string literal and doc line that still names an old spelling (`: -> `); run it before the v1.6.0 removal and when updating docs. diff --git a/cmd/commands/canon_engine_argv.go b/cmd/commands/canon_engine_argv.go index 55a8f08ca..89504ef85 100644 --- a/cmd/commands/canon_engine_argv.go +++ b/cmd/commands/canon_engine_argv.go @@ -72,6 +72,10 @@ func rewriteOldToNew(t *canonTableT, root *cobra.Command, args []string, start i if kind == "removed" { return nil, nil, removedError(best) } + // Identity breakouts keep their spelling and need no deprecation warning. + if strings.Join(best.From, " ") == strings.Join(best.To, " ") { + return args, nil, nil + } removal := best.RemovalAt if removal == "" { removal = canonRemovalDefault diff --git a/cmd/commands/canon_engine_argv_test.go b/cmd/commands/canon_engine_argv_test.go index 48f2e372a..10a1118b5 100644 --- a/cmd/commands/canon_engine_argv_test.go +++ b/cmd/commands/canon_engine_argv_test.go @@ -20,6 +20,30 @@ import ( func sp(s string) []string { return strings.Fields(s) } +func TestCanonEngineArgvIdentityBreakouts(t *testing.T) { + root := newFixtureTree() + for _, input := range []string{"ci", "ci nodes ls", "--json ci nodes ls", "functions", "functions deploy --force"} { + t.Run(input, func(t *testing.T) { + args := sp(input) + got, notes, err := rewriteCanonArgsWith(&canonTable, root, args, true) + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(got, args) { + t.Errorf("identity breakout changed argv: got %v, want %v", got, args) + } + if len(notes) != 0 { + t.Errorf("identity breakout emitted deprecation notes: %+v", notes) + } + }) + } + // The identity parent must not hide a longer, real move beneath it. + got, notes, err := rewriteCanonArgsWith(&canonTable, root, sp("ci eval run"), true) + if err != nil || strings.Join(got, " ") != "eval run" || len(notes) != 1 || notes[0].New != "eval" { + t.Fatalf("child breakout: argv=%v notes=%+v err=%v", got, notes, err) + } +} + func TestCanonEngineArgv(t *testing.T) { tb := fixtureTable(t) type tc struct{ in, v14, v15 string } From b391ae5a1fbe8d8a9376c89e7e73d68a523e6152 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Wed, 28 Oct 2026 08:39:06 -0400 Subject: [PATCH 2/8] fix(db): distinguish remote plugin flags from values (D-1151) --- .github/wiki/database-migrations.md | 2 + cmd/commands/db_remote_plugin.go | 42 ++++++++++++++++----- cmd/commands/db_remote_plugin_test.go | 54 +++++++++++++++++++++++++++ 3 files changed, 88 insertions(+), 10 deletions(-) diff --git a/.github/wiki/database-migrations.md b/.github/wiki/database-migrations.md index 8f0f5700e..17068c26b 100644 --- a/.github/wiki/database-migrations.md +++ b/.github/wiki/database-migrations.md @@ -59,6 +59,8 @@ Both `DELETE` statements execute inside the same transaction as the down DDL. Ei Both subcommands accept `--migration-dir ` to override auto-detection. On remote targets (`--env staging|prod`) the flag is forwarded to the remote CLI; the pre-flight version-drift check guarantees the remote binary understands it. +Remote migration commands validate `--plugin` locally before dispatch and forward it once. A directory value equal to `--plugin` remains a directory value and does not suppress plugin validation or forwarding. + ## Migration file naming | Pattern | Meaning | diff --git a/cmd/commands/db_remote_plugin.go b/cmd/commands/db_remote_plugin.go index 7e6bcbdc1..6fcc80e49 100644 --- a/cmd/commands/db_remote_plugin.go +++ b/cmd/commands/db_remote_plugin.go @@ -6,12 +6,16 @@ package commands // Outputs: the argv with "--plugin " appended, or the E401 error from // validatePluginFlag for an invalid name. // Constraints: no I/O. The argv is copied, never mutated in place. A caller -// that already forwards --plugin is left alone; the dedup matches the exact -// "--plugin" token because no builder emits the "--plugin=x" form. Quoting +// that already forwards --plugin is left alone; flag metadata distinguishes +// flags from their values during deduplication. Quoting // happens later in shellQuoteArgs. An invalid name fails before any SSH call. // SPORT: cli/cmd/commands — see D-0232, db_remote.go, plugin_flag.go. -import "github.com/spf13/cobra" +import ( + "strings" + + "github.com/spf13/cobra" +) // withRemotePluginFlag appends "--plugin " to remoteArgs when cmd carries // a non-empty --plugin value and the caller did not already forward it (D-0232). @@ -26,16 +30,34 @@ func withRemotePluginFlag(cmd *cobra.Command, remoteArgs []string) ([]string, er if f == nil || f.Value.String() == "" { return remoteArgs, nil } - for _, a := range remoteArgs { - if a == "--plugin" { - return remoteArgs, nil - } - } name := f.Value.String() if err := validatePluginFlag(name); err != nil { return nil, err } + insertAt := len(remoteArgs) + for i := 0; i < len(remoteArgs); i++ { + a := remoteArgs[i] + if a == "--" { + insertAt = i + break + } + if !strings.HasPrefix(a, "--") { + continue + } + flagName, _, inlineValue := strings.Cut(strings.TrimPrefix(a, "--"), "=") + if flagName == "plugin" { + return remoteArgs, nil + } + flag := cmd.Flags().Lookup(flagName) + if flag == nil { + flag = cmd.InheritedFlags().Lookup(flagName) + } + if flag != nil && flag.NoOptDefVal == "" && !inlineValue { + i++ // A required value may itself look like --plugin. + } + } out := make([]string, 0, len(remoteArgs)+2) - out = append(out, remoteArgs...) - return append(out, "--plugin", name), nil + out = append(out, remoteArgs[:insertAt]...) + out = append(out, "--plugin", name) + return append(out, remoteArgs[insertAt:]...), nil } diff --git a/cmd/commands/db_remote_plugin_test.go b/cmd/commands/db_remote_plugin_test.go index 83fd62357..e20c7b5fd 100644 --- a/cmd/commands/db_remote_plugin_test.go +++ b/cmd/commands/db_remote_plugin_test.go @@ -6,6 +6,7 @@ package commands import ( "context" + "reflect" "testing" "github.com/spf13/cobra" @@ -100,3 +101,56 @@ func TestWithRemotePluginFlag_NoDuplicateNoMutation(t *testing.T) { t.Fatalf("duplicate forwarded: %q, err = %v", again, err) } } + +func TestWithRemotePluginFlag_DistinguishesFlagsFromValues(t *testing.T) { + for _, tc := range []struct { + name string + args []string + forwarded bool + }{ + {"string value", []string{"--migration-dir", "--plugin"}, false}, + {"inline string value", []string{"--migration-dir=--plugin"}, false}, + {"boolean then plugin", []string{"--dry-run", "--plugin", "claw"}, true}, + {"inline plugin", []string{"--plugin=claw"}, true}, + } { + t.Run(tc.name, func(t *testing.T) { + cmd := &cobra.Command{Use: "up"} + cmd.Flags().String("plugin", "claw", "") + cmd.Flags().String("migration-dir", "", "") + cmd.Flags().Bool("dry-run", false, "") + out, err := withRemotePluginFlag(cmd, tc.args) + want := append([]string(nil), tc.args...) + if !tc.forwarded { + want = append(want, "--plugin", "claw") + } + if err != nil || !reflect.DeepEqual(out, want) { + t.Fatalf("out = %q, err = %v; want %q", out, err, want) + } + }) + } +} + +func TestWithRemotePluginFlag_PositionalPluginIsNotAFlag(t *testing.T) { + cmd := &cobra.Command{Use: "up"} + cmd.Flags().String("plugin", "claw", "") + out, err := withRemotePluginFlag(cmd, []string{"db", "migrate", "up", "--", "--plugin"}) + want := []string{"db", "migrate", "up", "--plugin", "claw", "--", "--plugin"} + if err != nil || !reflect.DeepEqual(out, want) { + t.Fatalf("out = %q, err = %v; want %q", out, err, want) + } +} + +func TestRemoteUp_PluginLikeDirectoryStillValidatesPlugin(t *testing.T) { + sent, err := remoteUp(t, map[string]string{"migration-dir": "--plugin", "plugin": "../x"}) + if err == nil || len(sent) != 0 { + t.Fatalf("err = %v, sent = %q; want validation error and no SSH", err, sent) + } +} + +func TestWithRemotePluginFlag_ValidatesAlreadyForwardedPlugin(t *testing.T) { + cmd := &cobra.Command{Use: "up"} + cmd.Flags().String("plugin", "../x", "") + if _, err := withRemotePluginFlag(cmd, []string{"--plugin", "../x"}); err == nil { + t.Fatal("already forwarded plugin bypassed local validation") + } +} From a22f58eab340a791632af6700f34bc888838e9ee Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Wed, 28 Oct 2026 13:08:24 -0400 Subject: [PATCH 3/8] refactor(canon): extract boolValue to satisfy 300-line file budget --- cmd/commands/canon_bool_value.go | 12 ++++++++++++ cmd/commands/canon_engine_argv.go | 7 ------- 2 files changed, 12 insertions(+), 7 deletions(-) create mode 100644 cmd/commands/canon_bool_value.go diff --git a/cmd/commands/canon_bool_value.go b/cmd/commands/canon_bool_value.go new file mode 100644 index 000000000..15d5dda32 --- /dev/null +++ b/cmd/commands/canon_bool_value.go @@ -0,0 +1,12 @@ +package commands + +import "github.com/spf13/pflag" + +// boolValue is a stand-in bool flag value for the implicit help flags. +type boolValue struct{} + +func (boolValue) String() string { return "false" } +func (boolValue) Set(string) error { return nil } +func (boolValue) Type() string { return "bool" } + +var _ pflag.Value = boolValue{} diff --git a/cmd/commands/canon_engine_argv.go b/cmd/commands/canon_engine_argv.go index 89504ef85..f8478d7c1 100644 --- a/cmd/commands/canon_engine_argv.go +++ b/cmd/commands/canon_engine_argv.go @@ -293,10 +293,3 @@ func rootFlag(root *cobra.Command, a string) *pflag.Flag { } return nil } - -// boolValue is a stand-in bool flag value for the implicit help flags. -type boolValue struct{} - -func (boolValue) String() string { return "false" } -func (boolValue) Set(string) error { return nil } -func (boolValue) Type() string { return "bool" } From efeda31431b82a5c1bfdecf57833d10a96c09417 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Wed, 28 Oct 2026 13:33:14 -0400 Subject: [PATCH 4/8] style(cmd): gofmt-align canon_bool_value methods --- cmd/commands/canon_bool_value.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/commands/canon_bool_value.go b/cmd/commands/canon_bool_value.go index 15d5dda32..51a6ad466 100644 --- a/cmd/commands/canon_bool_value.go +++ b/cmd/commands/canon_bool_value.go @@ -5,8 +5,8 @@ import "github.com/spf13/pflag" // boolValue is a stand-in bool flag value for the implicit help flags. type boolValue struct{} -func (boolValue) String() string { return "false" } +func (boolValue) String() string { return "false" } func (boolValue) Set(string) error { return nil } -func (boolValue) Type() string { return "bool" } +func (boolValue) Type() string { return "bool" } var _ pflag.Value = boolValue{} From 36f0f398ca3ba75445430e02957c6bfd31b9b25c Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Wed, 28 Oct 2026 16:44:30 -0400 Subject: [PATCH 5/8] refactor(cmd): move boolValue and plugin flag validation out of cmd/commands Move NoopBoolValue to internal/cmdregistry and inline plugin name validation at call sites, deleting canon_bool_value.go and plugin_flag.go from cmd/commands to satisfy the package-size ratchet. --- cmd/commands/canon_bool_value.go | 12 ------------ cmd/commands/canon_engine_argv.go | 3 ++- cmd/commands/db_hasura.go | 25 ++++++++++++++----------- cmd/commands/db_migrate.go | 27 +++++++++++++++------------ cmd/commands/db_remote_plugin.go | 8 ++++++-- cmd/commands/plugin_flag.go | 20 -------------------- internal/cmdregistry/flags.go | 11 +++++++++++ internal/plugin/name_validate.go | 12 ++++++------ 8 files changed, 54 insertions(+), 64 deletions(-) delete mode 100644 cmd/commands/canon_bool_value.go delete mode 100644 cmd/commands/plugin_flag.go diff --git a/cmd/commands/canon_bool_value.go b/cmd/commands/canon_bool_value.go deleted file mode 100644 index 51a6ad466..000000000 --- a/cmd/commands/canon_bool_value.go +++ /dev/null @@ -1,12 +0,0 @@ -package commands - -import "github.com/spf13/pflag" - -// boolValue is a stand-in bool flag value for the implicit help flags. -type boolValue struct{} - -func (boolValue) String() string { return "false" } -func (boolValue) Set(string) error { return nil } -func (boolValue) Type() string { return "bool" } - -var _ pflag.Value = boolValue{} diff --git a/cmd/commands/canon_engine_argv.go b/cmd/commands/canon_engine_argv.go index f8478d7c1..d1c65399a 100644 --- a/cmd/commands/canon_engine_argv.go +++ b/cmd/commands/canon_engine_argv.go @@ -23,6 +23,7 @@ import ( "strings" "github.com/spf13/cobra" + "github.com/nself-org/cli/internal/cmdregistry" "github.com/spf13/pflag" ) @@ -289,7 +290,7 @@ func rootFlag(root *cobra.Command, a string) *pflag.Flag { return f } if a == "--help" || a == "-h" || a == "--version" { - return &pflag.Flag{Name: strings.TrimLeft(a, "-"), Value: boolValue{}} + return &pflag.Flag{Name: strings.TrimLeft(a, "-"), Value: cmdregistry.NoopBoolValue{}} } return nil } diff --git a/cmd/commands/db_hasura.go b/cmd/commands/db_hasura.go index 184e0aa3e..28a9e3801 100644 --- a/cmd/commands/db_hasura.go +++ b/cmd/commands/db_hasura.go @@ -6,13 +6,14 @@ package commands // Constraints: split out of db.go (CLI-R12) as a pure move, no behavior change. import ( - "fmt" - "os" - "os/exec" +"fmt" +"os" +"os/exec" - "github.com/nself-org/cli/internal/database" +"github.com/nself-org/cli/internal/database" + "github.com/nself-org/cli/internal/plugin" - "github.com/spf13/cobra" +"github.com/spf13/cobra" ) func runDBHasuraConsole(cmd *cobra.Command, _ []string) error { @@ -90,17 +91,19 @@ func runDBHasuraMetadataReload(cmd *cobra.Command, _ []string) error { } func runDBVerifyChecksums(cmd *cobra.Command, _ []string) error { - plugin, _ := cmd.Flags().GetString("plugin") - if err := validatePluginFlag(plugin); err != nil { - return err +pluginName, _ := cmd.Flags().GetString("plugin") + if pluginName != "" { + if err := plugin.ValidateName(pluginName); err != nil { + return err + } } cfg, err := loadProjectConfig() if err != nil { return err } - mismatches, err := database.VerifyChecksums(cmd.Context(), cfg, plugin) - if err != nil { - return fmt.Errorf("verify checksums: %w", err) + mismatches, err := database.VerifyChecksums(cmd.Context(), cfg, pluginName) +if err != nil { +return fmt.Errorf("verify checksums: %w", err) } if len(mismatches) == 0 { fmt.Println("All checksums verified.") diff --git a/cmd/commands/db_migrate.go b/cmd/commands/db_migrate.go index 90c52e8e7..bcd72d73d 100644 --- a/cmd/commands/db_migrate.go +++ b/cmd/commands/db_migrate.go @@ -6,15 +6,16 @@ package commands // Constraints: split out of db.go (CLI-R12) as a pure move, no behavior change. import ( - "fmt" - "os" - "os/exec" - "path/filepath" - "time" +"fmt" +"os" +"os/exec" +"path/filepath" +"time" - "github.com/nself-org/cli/internal/database" +"github.com/nself-org/cli/internal/database" + "github.com/nself-org/cli/internal/plugin" - "github.com/spf13/cobra" +"github.com/spf13/cobra" ) func runDBMigrateUp(cmd *cobra.Command, _ []string) error { @@ -54,9 +55,11 @@ func runDBMigrateUp(cmd *cobra.Command, _ []string) error { return fmt.Errorf("PostgreSQL is not running — start with 'nself start' first") } - plugin, _ := cmd.Flags().GetString("plugin") - if err := validatePluginFlag(plugin); err != nil { - return err + pluginName, _ := cmd.Flags().GetString("plugin") + if pluginName != "" { + if err := plugin.ValidateName(pluginName); err != nil { + return err + } } // --migration-dir: apply all .sql files in the given directory (G-008). @@ -96,7 +99,7 @@ func runDBMigrateUp(cmd *cobra.Command, _ []string) error { // Default directory dry-run: PendingMigrations issues SELECTs only and // creates nothing (P7-PROD-84); it never reaches MigrateUp. if dryRun { - pending, err := database.PendingMigrations(cmd.Context(), cfg, plugin) + pending, err := database.PendingMigrations(cmd.Context(), cfg, pluginName) if err != nil { return fmt.Errorf("pending migrations: %w", err) } @@ -110,7 +113,7 @@ func runDBMigrateUp(cmd *cobra.Command, _ []string) error { return nil } - count, err := database.MigrateUp(cmd.Context(), cfg, plugin) + count, err := database.MigrateUp(cmd.Context(), cfg, pluginName) if err != nil { return fmt.Errorf("migrate up: %w", err) } diff --git a/cmd/commands/db_remote_plugin.go b/cmd/commands/db_remote_plugin.go index 6fcc80e49..ae0a208a3 100644 --- a/cmd/commands/db_remote_plugin.go +++ b/cmd/commands/db_remote_plugin.go @@ -15,6 +15,8 @@ import ( "strings" "github.com/spf13/cobra" + + "github.com/nself-org/cli/internal/plugin" ) // withRemotePluginFlag appends "--plugin " to remoteArgs when cmd carries @@ -31,8 +33,10 @@ func withRemotePluginFlag(cmd *cobra.Command, remoteArgs []string) ([]string, er return remoteArgs, nil } name := f.Value.String() - if err := validatePluginFlag(name); err != nil { - return nil, err + if name != "" { + if err := plugin.ValidateName(name); err != nil { + return nil, err + } } insertAt := len(remoteArgs) for i := 0; i < len(remoteArgs); i++ { diff --git a/cmd/commands/plugin_flag.go b/cmd/commands/plugin_flag.go deleted file mode 100644 index 71544a8df..000000000 --- a/cmd/commands/plugin_flag.go +++ /dev/null @@ -1,20 +0,0 @@ -package commands - -// plugin_flag.go — the shared --plugin flag check for db commands. -// -// Purpose: refuse a path-shaped --plugin value before any directory is read -// (P7-PLUG-20). Split out of plugin_lifecycle.go for the 300-line cap. -// Inputs: the --plugin value ("" means the project's own migrations). -// Outputs: nil, or the E401 error from plugin.ValidateName. -// Constraints: no I/O. - -import "github.com/nself-org/cli/internal/plugin" - -// validatePluginFlag checks a non-empty --plugin value with plugin.ValidateName, -// so a path-shaped name is refused before any directory is read (P7-PLUG-20). -func validatePluginFlag(name string) error { - if name == "" { - return nil - } - return plugin.ValidateName(name) -} diff --git a/internal/cmdregistry/flags.go b/internal/cmdregistry/flags.go index 7874eb8eb..9b0e7533b 100644 --- a/internal/cmdregistry/flags.go +++ b/internal/cmdregistry/flags.go @@ -68,3 +68,14 @@ func strPtr(s string) *string { } return &s } + +// NoopBoolValue is a stand-in bool flag value for implicit flags that should +// never be set by the user (e.g. suppressed help flags). It satisfies +// pflag.Value but ignores all writes. +type NoopBoolValue struct{} + +func (NoopBoolValue) String() string { return "false" } +func (NoopBoolValue) Set(string) error { return nil } +func (NoopBoolValue) Type() string { return "bool" } + +var _ pflag.Value = NoopBoolValue{} diff --git a/internal/plugin/name_validate.go b/internal/plugin/name_validate.go index e5b46bf88..c02c8d355 100644 --- a/internal/plugin/name_validate.go +++ b/internal/plugin/name_validate.go @@ -87,10 +87,10 @@ func safePublishedBinaryPath(bin string) (string, error) { // validateCLIBinaries validates every binary name the manifest would publish. func validateCLIBinaries(name string, m *PluginManifest) error { - for _, bin := range cliBinaryNames(name, m) { - if _, err := safePublishedBinaryPath(bin); err != nil { - return err - } - } - return nil +for _, bin := range cliBinaryNames(name, m) { +if _, err := safePublishedBinaryPath(bin); err != nil { +return err +} +} +return nil } From 589fb580bbdeab7501c80b61fb1f1607b99ad507 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Wed, 28 Oct 2026 17:53:03 -0400 Subject: [PATCH 6/8] style(cmd): gofmt canon_engine_argv, db_hasura, db_migrate, plugin name_validate --- cmd/commands/canon_engine_argv.go | 2 +- cmd/commands/db_hasura.go | 16 ++++++++-------- cmd/commands/db_migrate.go | 14 +++++++------- internal/plugin/name_validate.go | 12 ++++++------ 4 files changed, 22 insertions(+), 22 deletions(-) diff --git a/cmd/commands/canon_engine_argv.go b/cmd/commands/canon_engine_argv.go index d1c65399a..440327a90 100644 --- a/cmd/commands/canon_engine_argv.go +++ b/cmd/commands/canon_engine_argv.go @@ -22,8 +22,8 @@ package commands import ( "strings" - "github.com/spf13/cobra" "github.com/nself-org/cli/internal/cmdregistry" + "github.com/spf13/cobra" "github.com/spf13/pflag" ) diff --git a/cmd/commands/db_hasura.go b/cmd/commands/db_hasura.go index 28a9e3801..c9a7fc376 100644 --- a/cmd/commands/db_hasura.go +++ b/cmd/commands/db_hasura.go @@ -6,14 +6,14 @@ package commands // Constraints: split out of db.go (CLI-R12) as a pure move, no behavior change. import ( -"fmt" -"os" -"os/exec" + "fmt" + "os" + "os/exec" -"github.com/nself-org/cli/internal/database" + "github.com/nself-org/cli/internal/database" "github.com/nself-org/cli/internal/plugin" -"github.com/spf13/cobra" + "github.com/spf13/cobra" ) func runDBHasuraConsole(cmd *cobra.Command, _ []string) error { @@ -91,7 +91,7 @@ func runDBHasuraMetadataReload(cmd *cobra.Command, _ []string) error { } func runDBVerifyChecksums(cmd *cobra.Command, _ []string) error { -pluginName, _ := cmd.Flags().GetString("plugin") + pluginName, _ := cmd.Flags().GetString("plugin") if pluginName != "" { if err := plugin.ValidateName(pluginName); err != nil { return err @@ -102,8 +102,8 @@ pluginName, _ := cmd.Flags().GetString("plugin") return err } mismatches, err := database.VerifyChecksums(cmd.Context(), cfg, pluginName) -if err != nil { -return fmt.Errorf("verify checksums: %w", err) + if err != nil { + return fmt.Errorf("verify checksums: %w", err) } if len(mismatches) == 0 { fmt.Println("All checksums verified.") diff --git a/cmd/commands/db_migrate.go b/cmd/commands/db_migrate.go index bcd72d73d..ac55cdaff 100644 --- a/cmd/commands/db_migrate.go +++ b/cmd/commands/db_migrate.go @@ -6,16 +6,16 @@ package commands // Constraints: split out of db.go (CLI-R12) as a pure move, no behavior change. import ( -"fmt" -"os" -"os/exec" -"path/filepath" -"time" + "fmt" + "os" + "os/exec" + "path/filepath" + "time" -"github.com/nself-org/cli/internal/database" + "github.com/nself-org/cli/internal/database" "github.com/nself-org/cli/internal/plugin" -"github.com/spf13/cobra" + "github.com/spf13/cobra" ) func runDBMigrateUp(cmd *cobra.Command, _ []string) error { diff --git a/internal/plugin/name_validate.go b/internal/plugin/name_validate.go index c02c8d355..e5b46bf88 100644 --- a/internal/plugin/name_validate.go +++ b/internal/plugin/name_validate.go @@ -87,10 +87,10 @@ func safePublishedBinaryPath(bin string) (string, error) { // validateCLIBinaries validates every binary name the manifest would publish. func validateCLIBinaries(name string, m *PluginManifest) error { -for _, bin := range cliBinaryNames(name, m) { -if _, err := safePublishedBinaryPath(bin); err != nil { -return err -} -} -return nil + for _, bin := range cliBinaryNames(name, m) { + if _, err := safePublishedBinaryPath(bin); err != nil { + return err + } + } + return nil } From aeeb60853d701567d0faa38d0121203f4c257a4e Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Wed, 28 Oct 2026 18:10:55 -0400 Subject: [PATCH 7/8] docs(cmd): fix stale header in db_remote_plugin and drop redundant name guard --- cmd/commands/db_remote_plugin.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/cmd/commands/db_remote_plugin.go b/cmd/commands/db_remote_plugin.go index ae0a208a3..8b3a25dee 100644 --- a/cmd/commands/db_remote_plugin.go +++ b/cmd/commands/db_remote_plugin.go @@ -4,12 +4,12 @@ package commands // Inputs: the cobra command (may carry a --plugin flag) and the remote argv a // db subcommand built. // Outputs: the argv with "--plugin " appended, or the E401 error from -// validatePluginFlag for an invalid name. +// plugin.ValidateName for an invalid name. // Constraints: no I/O. The argv is copied, never mutated in place. A caller // that already forwards --plugin is left alone; flag metadata distinguishes // flags from their values during deduplication. Quoting // happens later in shellQuoteArgs. An invalid name fails before any SSH call. -// SPORT: cli/cmd/commands — see D-0232, db_remote.go, plugin_flag.go. +// SPORT: cli/cmd/commands — see D-0232, db_remote.go, internal/plugin/name_validate.go. import ( "strings" @@ -33,10 +33,8 @@ func withRemotePluginFlag(cmd *cobra.Command, remoteArgs []string) ([]string, er return remoteArgs, nil } name := f.Value.String() - if name != "" { - if err := plugin.ValidateName(name); err != nil { - return nil, err - } + if err := plugin.ValidateName(name); err != nil { + return nil, err } insertAt := len(remoteArgs) for i := 0; i < len(remoteArgs); i++ { From d8392408fdb58be9cc7dc61c0e4529fe7673ba41 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Thu, 29 Oct 2026 08:56:31 -0400 Subject: [PATCH 8/8] refactor(cmd): move argv scanning out of command package --- cmd/commands/db_remote_plugin.go | 27 ++++-------------------- internal/cmdregistry/flag_argv.go | 34 +++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 23 deletions(-) create mode 100644 internal/cmdregistry/flag_argv.go diff --git a/cmd/commands/db_remote_plugin.go b/cmd/commands/db_remote_plugin.go index 8b3a25dee..a4be88d3d 100644 --- a/cmd/commands/db_remote_plugin.go +++ b/cmd/commands/db_remote_plugin.go @@ -12,10 +12,9 @@ package commands // SPORT: cli/cmd/commands — see D-0232, db_remote.go, internal/plugin/name_validate.go. import ( - "strings" - "github.com/spf13/cobra" + "github.com/nself-org/cli/internal/cmdregistry" "github.com/nself-org/cli/internal/plugin" ) @@ -36,27 +35,9 @@ func withRemotePluginFlag(cmd *cobra.Command, remoteArgs []string) ([]string, er if err := plugin.ValidateName(name); err != nil { return nil, err } - insertAt := len(remoteArgs) - for i := 0; i < len(remoteArgs); i++ { - a := remoteArgs[i] - if a == "--" { - insertAt = i - break - } - if !strings.HasPrefix(a, "--") { - continue - } - flagName, _, inlineValue := strings.Cut(strings.TrimPrefix(a, "--"), "=") - if flagName == "plugin" { - return remoteArgs, nil - } - flag := cmd.Flags().Lookup(flagName) - if flag == nil { - flag = cmd.InheritedFlags().Lookup(flagName) - } - if flag != nil && flag.NoOptDefVal == "" && !inlineValue { - i++ // A required value may itself look like --plugin. - } + insertAt, forwarded := cmdregistry.FlagInsertionIndex(remoteArgs, "plugin", cmd.Flags(), cmd.InheritedFlags()) + if forwarded { + return remoteArgs, nil } out := make([]string, 0, len(remoteArgs)+2) out = append(out, remoteArgs[:insertAt]...) diff --git a/internal/cmdregistry/flag_argv.go b/internal/cmdregistry/flag_argv.go new file mode 100644 index 000000000..b9a7ece3a --- /dev/null +++ b/internal/cmdregistry/flag_argv.go @@ -0,0 +1,34 @@ +package cmdregistry + +import ( + "strings" + + "github.com/spf13/pflag" +) + +// FlagInsertionIndex finds the position before a positional separator where a +// flag can be inserted, and reports whether name is already forwarded. Required +// flag values are skipped even when they resemble flags. +func FlagInsertionIndex(args []string, name string, local, inherited *pflag.FlagSet) (int, bool) { + for i := 0; i < len(args); i++ { + a := args[i] + if a == "--" { + return i, false + } + if !strings.HasPrefix(a, "--") { + continue + } + flagName, _, inlineValue := strings.Cut(strings.TrimPrefix(a, "--"), "=") + if flagName == name { + return i, true + } + flag := local.Lookup(flagName) + if flag == nil { + flag = inherited.Lookup(flagName) + } + if flag != nil && flag.NoOptDefVal == "" && !inlineValue { + i++ + } + } + return len(args), false +}