From 829394625a436f289a3591b43024650b5bf5bc94 Mon Sep 17 00:00:00 2001 From: George Tsiolis Date: Thu, 2 Jul 2026 12:35:59 +0300 Subject: [PATCH] fix: make bash completion work without the bash-completion package MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cobra's generated bash completion script calls _get_comp_words_by_ref from the bash-completion package on both of its init paths, so on stock macOS (bash 3.2, no bash-completion) every Tab press failed with "_get_comp_words_by_ref: command not found" (DEVX-950). Prepend a guarded pure-bash fallback to the generated script (defined only when the package is absent, the git-completion.bash approach). The fallback re-joins COMP_WORDS pieces split at COMP_WORDBREAKS separators using COMP_LINE to recover adjacency, matching the package's reassembly semantics, and stays bash 3.2 compatible. Both the fallback and Cobra's script body are generated against the writer resolved at execution time; delegating to Cobra's own RunE would write the body to the writer captured at InitDefaultCompletionCmd time, splitting the two halves across destinations when SetOut is called after NewRootCmd. Also replace the bash completion help text: the default recommended 'source <(lstk completion bash)', which is a silent no-op on bash 3.2 — recommend 'eval "$(lstk completion bash)"' instead. --- CLAUDE.md | 4 + cmd/completion.go | 157 ++++++++++++++++++++ cmd/completion_test.go | 33 +++++ cmd/root.go | 1 + test/integration/completion_test.go | 216 ++++++++++++++++++++++++++++ 5 files changed, 411 insertions(+) create mode 100644 cmd/completion.go create mode 100644 cmd/completion_test.go create mode 100644 test/integration/completion_test.go diff --git a/CLAUDE.md b/CLAUDE.md index f5af977d..304dd30c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -176,6 +176,10 @@ The release job (`.github/workflows/ci.yml`) builds the npm packages with `gorel - Never print directly to stdout/stderr (e.g., `fmt.Fprintf(os.Stderr, …)`). For user-facing output, emit events through `output.Sink`. For internal diagnostics, use `log.Logger`. If neither is available (e.g., during logger setup), return errors to the caller and let them decide. - Do not call `config.Get()` from domain/business-logic packages. Instead, extract the values you need at the command boundary (`cmd/`) and pass them as explicit function arguments. This keeps domain functions testable without requiring Viper/config initialization. +# Shell Completion + +Cobra's generated bash completion script requires `_get_comp_words_by_ref` from the bash-completion package on both of its init paths, and stock macOS (bash 3.2) ships without that package — so completion failed with "command not found" on every Tab (DEVX-950). `selfContainBashCompletion` in `cmd/completion.go` wraps the autogenerated `completion bash` command to prepend a guarded pure-bash fallback (defined only when the package is absent, the git-completion.bash approach) and replaces the help text. The fallback body must stay bash 3.2 compatible (no `declare -A`, namerefs, `mapfile`, case-conversion expansions). It covers only `_get_comp_words_by_ref`; Cobra's script still calls bash-completion's `_filedir` for `ShellCompDirectiveFilterFileExt`/`ShellCompDirectiveFilterDirs` (`MarkFlagFilename`/`MarkFlagDirname`) and the ActiveHelp second-Tab path — lstk uses none of these today, so adopting one means growing the fallback. In docs/help, never recommend `source <(lstk completion bash)` — it is a silent no-op on bash 3.2; recommend `eval "$(lstk completion bash)"` instead. Zsh/fish/powershell scripts are self-contained upstream and untouched. + # CLI Help Text - Write command `Short`/`Long` as unbroken paragraphs (one line each, blank line between); never hard-wrap a sentence in source. `wrapText` in `cmd/help.go` re-wraps to the terminal width at render time and `lstk docs` reads the raw text, so manual breaks fight both. Indented lines (examples, aligned output) are left as-is. diff --git a/cmd/completion.go b/cmd/completion.go new file mode 100644 index 00000000..db5161bd --- /dev/null +++ b/cmd/completion.go @@ -0,0 +1,157 @@ +package cmd + +import ( + "fmt" + "io" + + "github.com/spf13/cobra" +) + +// bashCompletionFallback is prepended to Cobra's generated bash completion +// script. Both init paths of that script end up calling +// _get_comp_words_by_ref from the bash-completion package, but stock macOS +// ships bash 3.2 with no bash-completion at all, so every Tab press failed +// with "_get_comp_words_by_ref: command not found" (DEVX-950). Mirroring the +// approach git-completion.bash ships, this defines a minimal replacement only +// when the package is absent; with bash-completion (v1 or v2) installed the +// guard skips it. The body must stay bash 3.2 compatible. +// +// The fallback covers only _get_comp_words_by_ref — the paths lstk reaches +// today. Cobra's script still calls bash-completion's _filedir (undefined +// without the package) for ShellCompDirectiveFilterFileExt (MarkFlagFilename +// with extensions), ShellCompDirectiveFilterDirs (MarkFlagDirname), and the +// ActiveHelp second-Tab path, so adopting any of those requires growing the +// fallback (or avoiding the directive). +const bashCompletionFallback = `# lstk: self-contained fallback for the bash-completion package. +# The generated script below calls _get_comp_words_by_ref, which stock macOS +# (bash 3.2, no bash-completion package) does not provide. Define a minimal +# replacement when absent so completion works without extra packages. +if ! declare -F _get_comp_words_by_ref >/dev/null 2>&1; then + _get_comp_words_by_ref() { + local exclude="" i w issep attach="" preblank j=-1 + local line="$COMP_LINE" + local -a rebuilt=() + local rcword="$COMP_CWORD" + if [[ "$1" == "-n" ]]; then + exclude="$2" + shift 2 + fi + + # readline splits the command line at every character in + # COMP_WORDBREAKS ('=' and ':' included), so '--flag=value' arrives in + # COMP_WORDS as the three words '--flag', '=', 'value'. Rebuild the + # word list, re-joining pieces around the separators listed in + # $exclude, and track where the word under the cursor lands. Pieces + # re-join only when typed with no whitespace between them; COMP_WORDS + # is identical for '--flag=x' and '--flag = x', so adjacency has to be + # recovered from COMP_LINE. + for ((i = 0; i < ${#COMP_WORDS[@]}; i++)); do + w="${COMP_WORDS[$i]}" + preblank="" + if [[ "$line" == [[:blank:]]* ]]; then + preblank=1 + fi + line="${line#*"$w"}" + issep="" + if [[ -n "$w" && -n "$exclude" && -z "${w//[$exclude]/}" ]]; then + issep=1 + fi + # A separator (or the piece right after one) attaches to the + # previous word when adjacent on the typed line, except directly + # after the command name. + if [[ $j -ge 1 && -z "$preblank" && ( -n "$issep" || -n "$attach" ) ]]; then + rebuilt[$j]="${rebuilt[$j]}$w" + else + j=$((j + 1)) + rebuilt[$j]="$w" + fi + if [[ $i -eq $COMP_CWORD ]]; then + rcword=$j + fi + attach="$issep" + done + + while [[ $# -gt 0 ]]; do + case "$1" in + cur) + cur="${rebuilt[$rcword]}" + ;; + prev) + prev="" + if [[ $rcword -gt 0 ]]; then + prev="${rebuilt[$((rcword - 1))]}" + fi + ;; + words) + words=("${rebuilt[@]}") + ;; + cword) + cword="$rcword" + ;; + esac + shift + done + return 0 + } +fi + +` + +// selfContainBashCompletion replaces the autogenerated `completion bash` +// command's RunE so the emitted script carries bashCompletionFallback ahead +// of Cobra's output, removing the hard dependency on the bash-completion +// package (DEVX-950). Cobra's own RunE writes to a writer captured when +// InitDefaultCompletionCmd ran, so both halves are generated here against the +// writer resolved at execution time — otherwise SetOut after NewRootCmd would +// split them across two destinations. It also replaces the help text: Cobra's +// default recommends 'source <(lstk completion bash)', which is a silent +// no-op on macOS's stock bash 3.2, and states a package dependency that no +// longer holds. +func selfContainBashCompletion(completionCmd *cobra.Command) { + var bashCmd *cobra.Command + for _, sub := range completionCmd.Commands() { + if sub.Name() == "bash" { + bashCmd = sub + break + } + } + if bashCmd == nil { + return + } + + bashCmd.RunE = func(cmd *cobra.Command, args []string) error { + out := cmd.OutOrStdout() + if _, err := io.WriteString(out, bashCompletionFallback); err != nil { + return err + } + // Cobra registers --no-descriptions unless CompletionOptions turned + // it off; an absent flag means descriptions stay enabled. + noDesc := false + if v, err := cmd.Flags().GetBool("no-descriptions"); err == nil { + noDesc = v + } + return cmd.Root().GenBashCompletionV2(out, !noDesc) + } + + name := bashCmd.Root().Name() + bashCmd.Long = fmt.Sprintf(`Generate the autocompletion script for the bash shell. + +The script works with or without the 'bash-completion' package: when the package is absent (e.g. stock macOS bash), a bundled fallback is used instead. + +To load completions in your current shell session: + + eval "$(%[1]s completion bash)" + +To load completions for every new session, add the line above to ~/.bashrc (or ~/.bash_profile on macOS), or execute once: + +#### Linux: + + %[1]s completion bash > /etc/bash_completion.d/%[1]s + +#### macOS (with the bash-completion Homebrew package): + + %[1]s completion bash > $(brew --prefix)/etc/bash_completion.d/%[1]s + +You will need to start a new shell for this setup to take effect. +`, name) +} diff --git a/cmd/completion_test.go b/cmd/completion_test.go new file mode 100644 index 00000000..18e1404c --- /dev/null +++ b/cmd/completion_test.go @@ -0,0 +1,33 @@ +package cmd + +import ( + "testing" +) + +// TestCompletionBashWritesFallbackAndScriptToSameWriter guards the DEVX-950 +// wiring: the fallback prelude and Cobra's generated script must both reach +// the writer configured at execution time. Cobra captures its output writer +// when InitDefaultCompletionCmd runs (before SetOut is called here), so a +// prepend-and-delegate wrapper would send the two halves to different +// destinations. +func TestCompletionBashWritesFallbackAndScriptToSameWriter(t *testing.T) { + out, err := executeWithArgs(t, "completion", "bash") + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + assertContains(t, out, "_get_comp_words_by_ref()") + assertContains(t, out, "__start_lstk") + assertContains(t, out, "__complete") +} + +// TestCompletionBashNoDescriptionsFlagStillHonored verifies the wrapped RunE +// keeps Cobra's --no-descriptions behavior: the generated script requests +// completions via __completeNoDesc instead of __complete. +func TestCompletionBashNoDescriptionsFlagStillHonored(t *testing.T) { + out, err := executeWithArgs(t, "completion", "bash", "--no-descriptions") + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + assertContains(t, out, "_get_comp_words_by_ref()") + assertContains(t, out, "__completeNoDesc") +} diff --git a/cmd/root.go b/cmd/root.go index 1ee95575..e9bd0935 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -162,6 +162,7 @@ func NewRootCmd(cfg *env.Env, tel *telemetry.Client, logger log.Logger) *cobra.C root.InitDefaultCompletionCmd() if completionCmd, _, err := root.Find([]string{"completion"}); err == nil && completionCmd.Name() == "completion" { requireSubcommand(completionCmd) + selfContainBashCompletion(completionCmd) } return root diff --git a/test/integration/completion_test.go b/test/integration/completion_test.go new file mode 100644 index 00000000..d07d7701 --- /dev/null +++ b/test/integration/completion_test.go @@ -0,0 +1,216 @@ +package integration_test + +import ( + "fmt" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/localstack/lstk/test/integration/env" +) + +// stockBash returns the bash to test against, preferring /bin/bash (bash 3.2 +// on macOS, the environment DEVX-950 was reported on) over whatever is first +// in PATH. +func stockBash(t *testing.T) string { + t.Helper() + if runtime.GOOS == "windows" { + t.Skip("bash completion not applicable on Windows") + } + if _, err := os.Stat("/bin/bash"); err == nil { + return "/bin/bash" + } + bashPath, err := exec.LookPath("bash") + if err != nil { + t.Skip("bash not available") + } + return bashPath +} + +// runBashCompletionDriver generates the bash completion script, writes it and +// the given driver script to disk, and runs the driver in a bare bash +// (--noprofile --norc, minimal env) so nothing from the developer's machine — +// in particular the bash-completion package — can leak in. That bare shell is +// exactly what DEVX-950 reproduces: stock macOS ships bash 3.2 with no +// bash-completion, so the generated script must be self-contained. The driver +// receives the completion script path as $1. +func runBashCompletionDriver(t *testing.T, driver string) (stdout, stderr string, err error) { + t.Helper() + bashPath := stockBash(t) + + tmpHome := t.TempDir() + script, genStderr, err := runLstk(t, testContext(t), t.TempDir(), testEnvWithHome(tmpHome, ""), "completion", "bash") + require.NoError(t, err, "lstk completion bash failed: %s", genStderr) + + dir := t.TempDir() + scriptPath := filepath.Join(dir, "lstk-completion.bash") + require.NoError(t, os.WriteFile(scriptPath, []byte(script), 0o600)) + driverPath := filepath.Join(dir, "driver.bash") + require.NoError(t, os.WriteFile(driverPath, []byte(driver), 0o600)) + + binDir, err := filepath.Abs(filepath.Dir(binaryPath())) + require.NoError(t, err) + + cmd := exec.CommandContext(testContext(t), bashPath, "--noprofile", "--norc", driverPath, scriptPath) + cmd.Dir = dir + // The driver invokes `lstk __complete ...`; force the file keyring so + // that subprocess never probes the developer's system keyring. + cmd.Env = []string{ + "HOME=" + tmpHome, + "PATH=" + binDir + ":/usr/bin:/bin", + fmt.Sprintf("%s=file", env.Keyring), + } + var outBuf, errBuf strings.Builder + cmd.Stdout = &outBuf + cmd.Stderr = &errBuf + err = cmd.Run() + return outBuf.String(), errBuf.String(), err +} + +// completeInDriver simulates pressing Tab on the given command line state and +// prints the resulting COMPREPLY, one completion per line. compWords is a +// bash array literal (readline splits at COMP_WORDBREAKS characters, '=' and +// ':' included, so a typed '--flag=value' must be given as '--flag = value'). +func completeInDriver(compWords string, cword int, line string) string { + return fmt.Sprintf(`source "$1" || exit 1 +COMP_WORDS=(%s) +COMP_CWORD=%d +COMP_LINE=%q +COMP_POINT=%d +__start_lstk +status=$? +printf '%%s\n' "${COMPREPLY[@]}" +exit $status +`, compWords, cword, line, len(line)) +} + +// TestBashCompletionWorksWithoutBashCompletionPackage guards against +// DEVX-950: on stock macOS (bash 3.2, no bash-completion package) every Tab +// press failed with "_get_comp_words_by_ref: command not found" because the +// Cobra-generated script depends on that function from the bash-completion +// package on both of its init paths. +func TestBashCompletionWorksWithoutBashCompletionPackage(t *testing.T) { + t.Parallel() + + stdout, stderr, err := runBashCompletionDriver(t, completeInDriver("lstk st", 1, "lstk st")) + require.NoError(t, err, "completion attempt failed\nstdout: %s\nstderr: %s", stdout, stderr) + assert.NotContains(t, stderr, "command not found") + + completions := strings.Fields(stdout) + assert.Contains(t, completions, "start") + assert.Contains(t, completions, "status") + assert.Contains(t, completions, "stop") +} + +// TestBashCompletionAfterWhitespaceSeparatedFlagValue covers the shape that +// separates adjacency-aware reassembly from naive gluing: in +// 'lstk --config= st' readline delivers the same COMP_WORDS as for +// 'lstk --config=st', and only COMP_LINE reveals that 'st' is a separate word +// that should be completed to a subcommand. +func TestBashCompletionAfterWhitespaceSeparatedFlagValue(t *testing.T) { + t.Parallel() + + stdout, stderr, err := runBashCompletionDriver(t, completeInDriver(`lstk --config = st`, 3, "lstk --config= st")) + require.NoError(t, err, "completion attempt failed\nstdout: %s\nstderr: %s", stdout, stderr) + + completions := strings.Fields(stdout) + assert.Contains(t, completions, "start") + assert.Contains(t, completions, "status") + assert.Contains(t, completions, "stop") +} + +// TestBashCompletionReassemblesWordbreakSplits verifies the self-contained +// _get_comp_words_by_ref fallback matches the bash-completion package's +// semantics: pieces split at COMP_WORDBREAKS characters are re-joined only +// when they were typed with no whitespace between them, which has to be +// recovered from COMP_LINE since COMP_WORDS is identical either way. +func TestBashCompletionReassemblesWordbreakSplits(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + compWords string + cword int + line string + expect []string + }{ + { + name: "adjacent pieces re-join", + compWords: `lstk --config = ./cfg`, + cword: 3, + line: "lstk --config=./cfg", + expect: []string{"cur=--config=./cfg", "prev=lstk", "cword=1", "nwords=2"}, + }, + { + name: "word after separator-then-space stays separate", + compWords: `lstk --config = st`, + cword: 3, + line: "lstk --config= st", + expect: []string{"cur=st", "prev=--config=", "cword=2", "nwords=3"}, + }, + { + name: "whitespace-surrounded separator stays separate", + compWords: `lstk --config = ./x`, + cword: 3, + line: "lstk --config = ./x", + expect: []string{"cur=./x", "prev==", "cword=3", "nwords=4"}, + }, + { + name: "empty word after separator-then-space", + compWords: `lstk --config = ""`, + cword: 3, + line: "lstk --config= ", + expect: []string{"cur=", "prev=--config=", "cword=2", "nwords=3"}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + driver := fmt.Sprintf(`source "$1" || exit 1 +COMP_WORDS=(%s) +COMP_CWORD=%d +COMP_LINE=%q +COMP_POINT=%d +run_reassembly() { + local cur prev words cword + _get_comp_words_by_ref -n =: cur prev words cword || exit 1 + printf 'cur=%%s\n' "$cur" + printf 'prev=%%s\n' "$prev" + printf 'cword=%%s\n' "$cword" + printf 'nwords=%%s\n' "${#words[@]}" +} +run_reassembly +`, tc.compWords, tc.cword, tc.line, len(tc.line)) + + stdout, stderr, err := runBashCompletionDriver(t, driver) + require.NoError(t, err, "driver failed\nstdout: %s\nstderr: %s", stdout, stderr) + assert.NotContains(t, stderr, "command not found") + + lines := strings.Split(strings.TrimRight(stdout, "\n"), "\n") + assert.Equal(t, tc.expect, lines) + }) + } +} + +// TestBashCompletionFallbackYieldsToInstalledPackage verifies the fallback is +// guarded: when the bash-completion package already provides +// _get_comp_words_by_ref, sourcing the lstk script must not replace it. +func TestBashCompletionFallbackYieldsToInstalledPackage(t *testing.T) { + t.Parallel() + + driver := `_get_comp_words_by_ref() { echo "package version"; } +source "$1" || exit 1 +_get_comp_words_by_ref +` + stdout, stderr, err := runBashCompletionDriver(t, driver) + require.NoError(t, err, "driver failed\nstdout: %s\nstderr: %s", stdout, stderr) + assert.Contains(t, stdout, "package version") +}