From d65c10ec6d80cfee449e44894c9753dc36835fb9 Mon Sep 17 00:00:00 2001 From: George Tsiolis <120486+gtsiolis@users.noreply.github.com> Date: Mon, 29 Jun 2026 20:19:55 +0000 Subject: [PATCH 1/2] fix: exit non-zero on unknown subcommand of grouping parents Parent commands that only group subcommands (config, setup, volume, snapshot) had no RunE, so Cobra printed help and exited 0 for an unknown or missing subcommand (e.g. `lstk config bogus`). This made lstk report success on an invalid invocation. Add a requireSubcommand helper that sets cobra.NoArgs plus a help-printing RunE so a bare parent invocation still shows help (exit 0) while an unknown subcommand exits non-zero. Add integration tests covering both the new behavior and the unknown-flag/unknown-command cases. Generated with [Linear](https://linear.app/localstack/issue/DEVX-941/lstk-exits-with-code-0-on-failure#agent-session-0d671de0) Co-authored-by: linear-code[bot] <222613912+linear-code[bot]@users.noreply.github.com> --- CLAUDE.md | 2 + cmd/config.go | 1 + cmd/root.go | 12 ++++++ cmd/setup.go | 1 + cmd/snapshot.go | 1 + cmd/volume.go | 1 + test/integration/exit_code_test.go | 63 ++++++++++++++++++++++++++++++ 7 files changed, 81 insertions(+) create mode 100644 test/integration/exit_code_test.go diff --git a/CLAUDE.md b/CLAUDE.md index 4537c08b..cf911e27 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -64,6 +64,8 @@ When no config file exists, lstk creates one at `$HOME/.config/lstk/config.toml` Use `lstk config path` to print the resolved config file path currently in use. When adding a new command that depends on configuration, wire config initialization explicitly in that command (`PreRunE: initConfig`). Keep side-effect-free commands (e.g., `version`, `config path`) without config initialization. +A parent command that only groups subcommands (e.g. `config`, `setup`, `volume`, `snapshot`) must call `requireSubcommand(cmd)` (in `cmd/root.go`). Cobra otherwise prints help and exits 0 for an unknown/missing subcommand of a non-runnable parent; `requireSubcommand` sets `cobra.NoArgs` plus a help-printing `RunE` so a bare invocation still shows help (exit 0) while an unknown subcommand exits non-zero. + Created automatically on first run with defaults. Supports emulator types: `aws`, `snowflake`, and `azure`. Each `[[containers]]` block may set an optional `image` to override the default Docker Hub image (e.g. an internal registry mirror or a locally loaded offline image). `ContainerConfig.Image()` returns `image` as-is when it already carries a tag (so the separately-configured `tag` is dropped in that case), otherwise it appends `tag` (or `latest`); the default `localstack/:` is used when `image` is unset. diff --git a/cmd/config.go b/cmd/config.go index 4d1a102c..5ea444fe 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -14,6 +14,7 @@ func newConfigCmd(cfg *env.Env) *cobra.Command { Use: "config", Short: "Manage configuration", } + requireSubcommand(cmd) cmd.AddCommand(newConfigProfileCmd(cfg)) cmd.AddCommand(newConfigPathCmd()) return cmd diff --git a/cmd/root.go b/cmd/root.go index 2ec10282..1e116d8c 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -130,6 +130,18 @@ func NewRootCmd(cfg *env.Env, tel *telemetry.Client, logger log.Logger) *cobra.C return root } +// requireSubcommand configures a parent command that only groups subcommands so +// an unknown or missing subcommand exits non-zero instead of Cobra's default of +// printing help and exiting 0. Cobra only validates args (and so rejects unknown +// subcommands via cobra.NoArgs) when the command is runnable, hence the RunE that +// prints help for a bare invocation. +func requireSubcommand(cmd *cobra.Command) { + cmd.Args = cobra.NoArgs + cmd.RunE = func(c *cobra.Command, _ []string) error { + return c.Help() + } +} + func Execute(ctx context.Context) error { if len(os.Args) > 1 && os.Args[1] == telemetry.FlushCommandName { return runFlushTelemetry(ctx, os.Args[2:]) diff --git a/cmd/setup.go b/cmd/setup.go index c38ef96e..65258b87 100644 --- a/cmd/setup.go +++ b/cmd/setup.go @@ -18,6 +18,7 @@ func newSetupCmd(cfg *env.Env) *cobra.Command { Short: "Set up emulator CLI integration", Long: "Set up emulator CLI integration for AWS or Azure.", } + requireSubcommand(cmd) cmd.AddCommand(newSetupAWSCmd(cfg)) cmd.AddCommand(newSetupAzureCmd(cfg)) return cmd diff --git a/cmd/snapshot.go b/cmd/snapshot.go index 00862e61..7e801205 100644 --- a/cmd/snapshot.go +++ b/cmd/snapshot.go @@ -91,6 +91,7 @@ func newSnapshotCmd(cfg *env.Env, tel *telemetry.Client, logger log.Logger) *cob Use: "snapshot", Short: "Manage emulator snapshots", } + requireSubcommand(cmd) cmd.AddCommand(newSnapshotSaveCmd(cfg)) cmd.AddCommand(newSnapshotLoadCmd(cfg, tel, logger)) cmd.AddCommand(newSnapshotListCmd(cfg, logger)) diff --git a/cmd/volume.go b/cmd/volume.go index 802271e1..1791c6e8 100644 --- a/cmd/volume.go +++ b/cmd/volume.go @@ -17,6 +17,7 @@ func newVolumeCmd(cfg *env.Env) *cobra.Command { Use: "volume", Short: "Manage emulator volume", } + requireSubcommand(cmd) cmd.AddCommand(newVolumePathCmd(cfg)) cmd.AddCommand(newVolumeClearCmd(cfg)) return cmd diff --git a/test/integration/exit_code_test.go b/test/integration/exit_code_test.go new file mode 100644 index 00000000..582d7c9c --- /dev/null +++ b/test/integration/exit_code_test.go @@ -0,0 +1,63 @@ +package integration_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestInvalidUsageExitsNonZero guards against regressions of DEVX-941, where +// lstk exited 0 even when a command failed. It covers two failure classes that +// must surface a non-zero exit code: an unknown flag, and an unknown subcommand +// of a parent command that only groups subcommands (config/setup/volume/ +// snapshot). The latter used to exit 0 because Cobra prints help and returns +// nil for an unrecognized subcommand of a non-runnable parent. +func TestInvalidUsageExitsNonZero(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + args []string + wantText string + }{ + {"unknown flag on start", []string{"start", "--bogus-flag-xyz"}, "unknown flag"}, + {"unknown flag on root", []string{"--bogus-flag-xyz"}, "unknown flag"}, + {"unknown command", []string{"bogus-command"}, "unknown command"}, + {"unknown config subcommand", []string{"config", "bogus"}, `unknown command "bogus"`}, + {"unknown setup subcommand", []string{"setup", "bogus"}, `unknown command "bogus"`}, + {"unknown volume subcommand", []string{"volume", "bogus"}, `unknown command "bogus"`}, + {"unknown snapshot subcommand", []string{"snapshot", "bogus"}, `unknown command "bogus"`}, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + e := testEnvWithHome(t.TempDir(), "") + _, stderr, err := runLstk(t, testContext(t), t.TempDir(), e, tc.args...) + require.Error(t, err, "expected %v to fail", tc.args) + requireExitCode(t, 1, err) + assert.Contains(t, stderr, tc.wantText) + }) + } +} + +// TestBareParentCommandExitsZero confirms that invoking a subcommand-grouping +// parent with no subcommand still prints help and exits 0 — the fix for +// DEVX-941 must reject unknown subcommands without breaking bare invocations. +func TestBareParentCommandExitsZero(t *testing.T) { + t.Parallel() + + for _, parent := range []string{"config", "setup", "volume", "snapshot"} { + parent := parent + t.Run(parent, func(t *testing.T) { + t.Parallel() + e := testEnvWithHome(t.TempDir(), "") + stdout, _, err := runLstk(t, testContext(t), t.TempDir(), e, parent) + require.NoError(t, err) + requireExitCode(t, 0, err) + assert.Contains(t, stdout, "Usage:") + }) + } +} From 7be782002ddc6fedbddce21f966ad234eea12409 Mon Sep 17 00:00:00 2001 From: George Tsiolis Date: Tue, 30 Jun 2026 14:17:41 +0300 Subject: [PATCH 2/2] fix: exit non-zero on unknown completion subcommand Cobra's autogenerated `completion` command is itself a subcommand-grouping parent (bash/zsh/fish/powershell) with no RunE, so an unknown shell such as `lstk completion bogus` hit the same exit-0 path as the other grouping parents: it printed help and exited 0. Unlike config/setup/volume/snapshot, the completion command is created lazily during Execute, so materialize it via InitDefaultCompletionCmd() before applying requireSubcommand. Covers the `completion bogusshell` case from the original report and adds it to the exit-code regression tests. --- CLAUDE.md | 2 +- cmd/root.go | 9 +++++++++ test/integration/exit_code_test.go | 8 +++++--- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index cf911e27..3c70f62c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -64,7 +64,7 @@ When no config file exists, lstk creates one at `$HOME/.config/lstk/config.toml` Use `lstk config path` to print the resolved config file path currently in use. When adding a new command that depends on configuration, wire config initialization explicitly in that command (`PreRunE: initConfig`). Keep side-effect-free commands (e.g., `version`, `config path`) without config initialization. -A parent command that only groups subcommands (e.g. `config`, `setup`, `volume`, `snapshot`) must call `requireSubcommand(cmd)` (in `cmd/root.go`). Cobra otherwise prints help and exits 0 for an unknown/missing subcommand of a non-runnable parent; `requireSubcommand` sets `cobra.NoArgs` plus a help-printing `RunE` so a bare invocation still shows help (exit 0) while an unknown subcommand exits non-zero. +A parent command that only groups subcommands (e.g. `config`, `setup`, `volume`, `snapshot`) must call `requireSubcommand(cmd)` (in `cmd/root.go`). Cobra otherwise prints help and exits 0 for an unknown/missing subcommand of a non-runnable parent; `requireSubcommand` sets `cobra.NoArgs` plus a help-printing `RunE` so a bare invocation still shows help (exit 0) while an unknown subcommand exits non-zero. Cobra's autogenerated `completion` command is the same shape, but it is created lazily during `Execute`, so `NewRootCmd` calls `root.InitDefaultCompletionCmd()` to materialize it before applying `requireSubcommand` (the call is idempotent — Cobra skips re-adding it). Created automatically on first run with defaults. Supports emulator types: `aws`, `snowflake`, and `azure`. diff --git a/cmd/root.go b/cmd/root.go index 1e116d8c..7f0765d1 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -127,6 +127,15 @@ func NewRootCmd(cfg *env.Env, tel *telemetry.Client, logger log.Logger) *cobra.C root.SetHelpCommandGroupID(groupCommands) root.SetCompletionCommandGroupID(groupCommands) + // Cobra's autogenerated completion command is itself a subcommand-grouping + // parent (bash/zsh/fish/powershell) with no RunE, so an unknown shell (e.g. + // `lstk completion bogus`) hits the same exit-0 path requireSubcommand fixes. + // It is created lazily during Execute, so force it now to wire it up too. + root.InitDefaultCompletionCmd() + if completionCmd, _, err := root.Find([]string{"completion"}); err == nil && completionCmd.Name() == "completion" { + requireSubcommand(completionCmd) + } + return root } diff --git a/test/integration/exit_code_test.go b/test/integration/exit_code_test.go index 582d7c9c..04367eaa 100644 --- a/test/integration/exit_code_test.go +++ b/test/integration/exit_code_test.go @@ -11,8 +11,9 @@ import ( // lstk exited 0 even when a command failed. It covers two failure classes that // must surface a non-zero exit code: an unknown flag, and an unknown subcommand // of a parent command that only groups subcommands (config/setup/volume/ -// snapshot). The latter used to exit 0 because Cobra prints help and returns -// nil for an unrecognized subcommand of a non-runnable parent. +// snapshot, plus Cobra's autogenerated completion). The latter used to exit 0 +// because Cobra prints help and returns nil for an unrecognized subcommand of a +// non-runnable parent. func TestInvalidUsageExitsNonZero(t *testing.T) { t.Parallel() @@ -28,6 +29,7 @@ func TestInvalidUsageExitsNonZero(t *testing.T) { {"unknown setup subcommand", []string{"setup", "bogus"}, `unknown command "bogus"`}, {"unknown volume subcommand", []string{"volume", "bogus"}, `unknown command "bogus"`}, {"unknown snapshot subcommand", []string{"snapshot", "bogus"}, `unknown command "bogus"`}, + {"unknown completion shell", []string{"completion", "bogus"}, `unknown command "bogus"`}, } for _, tc := range cases { @@ -49,7 +51,7 @@ func TestInvalidUsageExitsNonZero(t *testing.T) { func TestBareParentCommandExitsZero(t *testing.T) { t.Parallel() - for _, parent := range []string{"config", "setup", "volume", "snapshot"} { + for _, parent := range []string{"config", "setup", "volume", "snapshot", "completion"} { parent := parent t.Run(parent, func(t *testing.T) { t.Parallel()