fix: exit non-zero on unknown subcommand of grouping parents#346
Merged
Conversation
anisaoshafi
added
semver: patch
docs: skip
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>
gtsiolis
force-pushed
the
devx-941-lstk-exits-with-code-0-on-failure-50a1
branch
from
8bce2d1 to
ebd0abb
Compare
gtsiolis
changed the title
gtsiolis
force-pushed
the
devx-941-lstk-exits-with-code-0-on-failure-50a1
branch
from
ebd0abb to
d65c10e
Compare
gtsiolis
changed the title
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.
anisaoshafi
approved these changes
Jun 30, 2026
anisaoshafi
left a comment
anisaoshafi
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for fixing this 👏🏼 makes sense!
peter-smith-phd
added a commit
that referenced
this pull request
Jun 30, 2026
…rrors to stderr Telemetry (incorporates PR feedback): extension invocations now reach the analytics pipeline / data warehouse, not just the OTel tracing exporter. dispatchExtension emits an `lstk_command` event named `ext:<name>` (duration + exit code) for a resolved extension, so the warehouse tracks which extension ran. instrumentCommands skips its generic emit for the extension-dispatch path so the run is no longer mislabeled as `start`, and an unresolved command records nothing. extension.Invoke keeps its OTel span for tracing. CI fixes (failures after merging main #346): - The unknown-command error now goes to stderr (matching Cobra's own output), so `lstk <unknown>` puts "unknown command" on stderr and exits 1 — fixes TestInvalidUsageExitsNonZero/unknown_command and the in-tree TestExtensionUnknownCommandNoExtensionErrors. - The self-authorization integration tests strip the inherited LOCALSTACK_AUTH_TOKEN (CI sets one) so the "without token" path truly conveys no token and the extension refuses (exit 13). - Add TestExtensionInvocationRecordedInTelemetry asserting the `ext:<name>` event (and that it is not labeled `start`). Updates design Decision 8, the extension-runtime-context telemetry requirement, and task 3.6 to distinguish product telemetry (warehouse) from OTel tracing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Parent commands that only group subcommands (
config,setup,volume,snapshot) had noRunE, so Cobra printed help and exited 0 for an unknown or missing subcommand — e.g.lstk config bogusandlstk setup bogusreported success despite being invalid usage. Unknown top-level commands and unknown flags already exited non-zero; the bug was specific to subcommands of these non-runnable grouping parents.The fix adds a
requireSubcommandhelper (cmd/root.go) that setscobra.NoArgsplus a help-printingRunEon these parents. Cobra skips arg validation on a non-runnable parent, so theRunEis the load-bearing part — it makes the command runnable soNoArgsactually rejects the stray subcommand. A bare invocation (lstk config) still prints help and exits 0, while an unknown subcommand now exits 1 withError: unknown command "bogus" for "lstk config"on stderr.Cobra's autogenerated
completioncommand is the same shape — a non-runnable parent groupingbash/zsh/fish/powershell— solstk completion bogushad the identical exit-0 bug. Since that command is created lazily, lstk now materializes it (InitDefaultCompletionCmd) and appliesrequireSubcommandto it as well.The issue's literal example (
lstk start --bogus-flag-xyz, an unknown flag) already exited non-zero; the regression test covers it alongside the parent-command cases to lock the behavior down.Verified manually and with integration tests in
test/integration/exit_code_test.go: unknown flag, unknown command, and an unknown subcommand of every grouping parent (config/setup/volume/snapshot/completion) → exit 1; bare parent → exit 0.Note
A sibling exit-0 bug found while investigating this —
lstk setup awsexiting 0 when it can't write the profile — is split out to #349 (DEVX-944) to keep this PR focused on invalid-usage exit codes. The npm-launcher half of the report (the wrapper swallowing the binary's exit code) is handled separately in #347 (DEVX-942).