From 5705913ac53f0ad6c1c7a4f39c670894a32daa2d Mon Sep 17 00:00:00 2001 From: Joel Scheuner Date: Fri, 4 Sep 2026 10:24:13 +0200 Subject: [PATCH 1/4] Point users at shell completion setup on first start Co-Authored-By: Claude --- CLAUDE.md | 2 + cmd/root.go | 8 ++ internal/ui/run.go | 35 +++++ test/integration/completion_tip_test.go | 166 ++++++++++++++++++++++++ 4 files changed, 211 insertions(+) create mode 100644 test/integration/completion_tip_test.go diff --git a/CLAUDE.md b/CLAUDE.md index e6e44c6d..960e6f2c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -220,6 +220,8 @@ The release job (`.github/workflows/ci.yml`) builds the npm packages with `gorel 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. +Only Homebrew installs get completion wired up automatically (`homebrew_casks.completions` in `.goreleaser.yaml`), so the first successful *interactive* start emits a one-line `> Tip:` pointing at `lstk completion [bash|zsh|fish|powershell]` and the docs section above (same prefix and severity as `tipsForType`, whose tip renders directly above it). It is deliberately just a pointer — lstk does not write to the user's shell config, and there is no `--write`/install flag (rationale on the `completionTip` const in `internal/ui/run.go`, which also explains why the trigger is the first run rather than install time). Adding a second such nudge means gating it the same way: `firstRun` only, interactive only, after the emulator is up. + `lstk aws ` completes AWS services/operations/parameters by delegating to the AWS CLI's own `aws_completer` from a `ValidArgsFunction` on the `aws` command (DEVX-846) — `awscli.Complete` in `internal/awscli/complete.go`, wired in `cmd/aws.go`. Going through Cobra rather than registering `complete -C aws_completer lstk` is what makes it work in every shell `lstk completion` supports (the native registration is bash/zsh-only) and keeps the bash fallback above unchanged. `aws_completer` speaks bash's `complete -C` protocol: `COMP_LINE`/`COMP_POINT` in, candidates one-per-line out. Two constraints it imposes: the line must start with the literal word `aws` (the completer drops the first word before matching, so `lstk aws s3 l` returns nothing), and `COMP_POINT` is a **character** offset, not a byte one. Cobra never runs `PreRunE` on the `__complete` path, so completion stays offline — no config load, no Docker health check, no endpoint resolution — which matches the completer, which never contacts an endpoint. A missing or failing completer must return `ShellCompDirectiveDefault` and print nothing: any output on this path is read by the shell as a candidate. The Tab-press timeout is set by the caller in `cmd/aws.go` (`awsCompletionTimeout`), not inside `awscli.Complete` — an internal deadline made the unit test flaky on cold CI runs. That deadline only actually bounds a Tab press because `Complete` also sets `cmd.WaitDelay`: `exec.CommandContext` kills the completer on expiry but does not close a pipe the completer's own children still hold, so a completer leaving a grandchild on stdout (a wrapper script that forks instead of exec'ing — what `/bin/sh` does on Linux, where it is dash) made `cmd.Output()` block for the grandchild's full lifetime regardless of the context. Keep `WaitDelay` set on any similar short captured-output exec. `lstk az` needs the same treatment but a different protocol (argcomplete: `_ARGCOMPLETE=1`, output on fd 8). # CLI Help Text diff --git a/cmd/root.go b/cmd/root.go index 5917e166..b421f6dc 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -351,6 +351,13 @@ func startEmulator(ctx context.Context, rt runtime.Runtime, cfg *env.Env, tel *t logger.Info("could not resolve friendly config path: %v", err) } + // Captured before ApplyEmulatorType clears firstRun: `lstk start --type aws` + // on a fresh install is still the user's first run, and the completion tip + // should fire there too. firstRun itself is only cleared below to skip the + // emulator picker and the default-emulator notice, both of which --type has + // already answered. + wasFirstRun := firstRun + // Apply the --type flag before resolving snapshot and start options so // everything downstream reflects the selected emulator. Uses the caller's // sink even in interactive mode, since the config mutation has to happen @@ -394,6 +401,7 @@ func startEmulator(ctx context.Context, rt runtime.Runtime, cfg *env.Env, tel *t ConfigPath: configPath, EmulatorLabel: config.CachedPlanLabel(), NeedsEmulatorSelection: firstRun, + CompletionTip: wasFirstRun, PostStart: autoLoad, }) } diff --git a/internal/ui/run.go b/internal/ui/run.go index 06958d7d..8ff9a327 100644 --- a/internal/ui/run.go +++ b/internal/ui/run.go @@ -41,8 +41,40 @@ type RunOptions struct { // auto-load a configured snapshot). It is skipped when the emulator was // already running. PostStart func(ctx context.Context, sink output.Sink) error + // CompletionTip points the user at the documented shell-completion setup + // once the emulator is up. The caller decides when it applies (the first + // run, i.e. config.toml was absent) so this package holds no policy; see + // completionTip for why the first run is the trigger. + CompletionTip bool } +// completionTip nudges users toward the completion scripts lstk already ships. +// Only Homebrew installs get them set up automatically (`.goreleaser.yaml`'s +// homebrew_casks.completions), so npm and GitHub-release users have to find the +// docs themselves and most never do. +// +// The trigger is the first run rather than install time, because no install +// path offers a usable hook: the npm package.json is generated by +// goreleaser-npm-publisher, a postinstall that edits a shell rc is hostile and +// is skipped under --ignore-scripts, and binary installs have no hook at all. +// The first run needs no new persisted state either — it means "config.toml was +// absent", and that same path creates the config, so the tip cannot repeat. +// +// It is emitted after the emulator is up, not before, so it does not become a +// third interruption ahead of the emulator picker and the update prompt. It is +// a plain inline MessageEvent rather than a DeferredEvent: Run does not render +// DeferredOutput (only runWithTUI does), so a deferred event would be silently +// dropped on this path. Emitting it immediately before runDoneMsg is safe +// because both sends cross the same ordered channel, and buffered lines are +// always flushed before the app quits. +// +// The literal "> Tip: " prefix and SeveritySecondary match tipsForType in +// internal/container/start.go, whose tip lands immediately above this one in +// the post-start block — SeverityNote would render "> Note:" there instead and +// read as a second, unrelated idiom. +const completionTip = "> Tip: Tab completion is available using lstk completion [bash|zsh|fish|powershell] " + + "as documented at https://docs.localstack.cloud/aws/developer-tools/running-localstack/lstk/#shell-completions" + func Run(parentCtx context.Context, runOpts RunOptions) error { ctx, cancel := context.WithCancel(parentCtx) defer cancel() @@ -138,6 +170,9 @@ func Run(parentCtx context.Context, runOpts RunOptions) error { } else { go container.ResolveAndCacheLabel(ctx, runOpts.StartOptions, result.Version, labelCh) } + if runOpts.CompletionTip { + sink.Emit(output.MessageEvent{Severity: output.SeveritySecondary, Text: completionTip}) + } p.Send(runDoneMsg{}) }() diff --git a/test/integration/completion_tip_test.go b/test/integration/completion_tip_test.go new file mode 100644 index 00000000..dbf3d1e2 --- /dev/null +++ b/test/integration/completion_tip_test.go @@ -0,0 +1,166 @@ +package integration_test + +import ( + "encoding/json" + "os" + "path/filepath" + "testing" + "time" + + "github.com/localstack/lstk/test/integration/env" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// completionTipText is the user-visible tip lstk shows once, after the first +// successful interactive start, pointing at the documented shell-completion +// setup. Asserted verbatim, including the "> Tip: " prefix: that prefix is the +// convention the neighbouring post-start tips use (tipsForType in +// internal/container/start.go), so it is part of the observable behavior rather +// than styling. +const completionTipText = "> Tip: Tab completion is available using lstk completion [bash|zsh|fish|powershell] " + + "as documented at https://docs.localstack.cloud/aws/developer-tools/running-localstack/lstk/#shell-completions" + +// firstRunHome returns an isolated home with no lstk config, so the run under +// test is a first run (config.toml absent is what firstRun means). +func firstRunHome(t *testing.T) (env.Environ, string) { + t.Helper() + + tmpHome := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(tmpHome, ".config"), 0755)) + e := env.Environ(testEnvWithHome(tmpHome, tmpHome)).With(env.DisableEvents, "1") + + configPath, _, err := runLstk(t, testContext(t), "", e, "config", "path") + require.NoError(t, err) + require.NoFileExists(t, configPath, "test setup: config must be absent for this to be a first run") + + return e, configPath +} + +func TestFirstRunShowsCompletionTip(t *testing.T) { + requireDocker(t) + _ = env.Require(t, env.AuthToken) + + cleanup() + t.Cleanup(cleanup) + + mockServer := createMockLicenseServer(true) + defer mockServer.Close() + + e, _ := firstRunHome(t) + + p := startLstkInPTY(t, testContext(t), e.With(env.APIEndpoint, mockServer.URL), "start") + + // First run shows the emulator picker; accept the highlighted default (AWS). + p.waitForOutput("Which emulator would you like to use?", "emulator selection prompt should appear on first run") + p.write("\r") + + // Post-start setup asks about the AWS CLI profile in the isolated home; decline it. + p.waitForOutputTimeout(awsSetupPrompt, 2*time.Minute, "container should become ready") + p.write("n") + + out, err := p.wait() + require.NoError(t, err, "lstk start should exit successfully") + + assert.Contains(t, out, completionTipText, "first successful interactive start should point at shell completion setup") +} + +// --type is the non-interactive answer to the first-run emulator picker, so it +// suppresses the picker — but the run is still the user's first, and the tip +// must survive that. +func TestFirstRunWithEmulatorTypeFlagShowsCompletionTip(t *testing.T) { + requireDocker(t) + _ = env.Require(t, env.AuthToken) + + cleanup() + t.Cleanup(cleanup) + + mockServer := createMockLicenseServer(true) + defer mockServer.Close() + + e, _ := firstRunHome(t) + + p := startLstkInPTY(t, testContext(t), e.With(env.APIEndpoint, mockServer.URL), "start", "--type", "aws") + + assert.NotContains(t, p.output(), "Which emulator would you like to use?", + "--type should answer the picker rather than showing it") + + p.waitForOutputTimeout(awsSetupPrompt, 2*time.Minute, "container should become ready") + p.write("n") + + out, err := p.wait() + require.NoError(t, err, "lstk start --type aws should exit successfully") + + assert.Contains(t, out, completionTipText, "--type must not suppress the first-run tip along with the picker") +} + +func TestSubsequentRunDoesNotShowCompletionTip(t *testing.T) { + requireDocker(t) + _ = env.Require(t, env.AuthToken) + + cleanup() + t.Cleanup(cleanup) + + mockServer := createMockLicenseServer(true) + defer mockServer.Close() + + e, configPath := firstRunHome(t) + + // Pre-create the config so this is no longer a first run. + require.NoError(t, os.MkdirAll(filepath.Dir(configPath), 0755)) + require.NoError(t, os.WriteFile(configPath, + []byte("[[containers]]\ntype = \"aws\"\ntag = \"latest\"\nport = \"4566\"\n"), 0644)) + + p := startLstkInPTY(t, testContext(t), e.With(env.APIEndpoint, mockServer.URL), "start") + + p.waitForOutputTimeout(awsSetupPrompt, 2*time.Minute, "container should become ready") + p.write("n") + + out, err := p.wait() + require.NoError(t, err, "lstk start should exit successfully") + + assert.NotContains(t, out, completionTipText, "the tip must not repeat once lstk has been configured") +} + +func TestFirstRunNonInteractiveShowsNoCompletionTip(t *testing.T) { + requireDocker(t) + _ = env.Require(t, env.AuthToken) + + cleanup() + t.Cleanup(cleanup) + + mockServer := createMockLicenseServer(true) + defer mockServer.Close() + + e, _ := firstRunHome(t) + + stdout, stderr, err := runLstk(t, testContext(t), "", e.With(env.APIEndpoint, mockServer.URL), "start") + require.NoError(t, err, "lstk start failed: %s", stderr) + + assert.Contains(t, stdout, "Configured with default emulator", "test setup: this should be a first run") + assert.NotContains(t, stdout, completionTipText, "a tip is noise in non-interactive output") +} + +func TestFirstRunJSONEnvelopeHasNoCompletionTip(t *testing.T) { + requireDocker(t) + _ = env.Require(t, env.AuthToken) + + cleanup() + t.Cleanup(cleanup) + + mockServer := createMockLicenseServer(true) + defer mockServer.Close() + + e, _ := firstRunHome(t) + + stdout, stderr, err := runLstk(t, testContext(t), "", e.With(env.APIEndpoint, mockServer.URL), "start", "--json") + require.NoError(t, err, "lstk start --json failed: %s", stderr) + + assert.NotContains(t, stdout, "Tab completion", "the tip must never reach machine-readable output") + + envelope := decodeEnvelope(t, stdout) + assert.Equal(t, "ok", envelope.Status) + var data startJSONData + require.NoError(t, json.Unmarshal(envelope.Data, &data)) + assert.Equal(t, "aws", data.Emulator, "test setup: the first run should have started the default emulator") +} From 069f1ca8020e6736b0477f717d0c7d32461f021e Mon Sep 17 00:00:00 2001 From: Joel Scheuner Date: Fri, 4 Sep 2026 12:05:16 +0200 Subject: [PATCH 2/4] Clean up root-owned emulator volume files in completion tip tests Co-Authored-By: Claude --- test/integration/awsconfig_test.go | 12 +----------- test/integration/completion_tip_test.go | 3 +++ test/integration/main_test.go | 24 ++++++++++++++++++++++++ 3 files changed, 28 insertions(+), 11 deletions(-) diff --git a/test/integration/awsconfig_test.go b/test/integration/awsconfig_test.go index ee0d1a46..fb509005 100644 --- a/test/integration/awsconfig_test.go +++ b/test/integration/awsconfig_test.go @@ -2,7 +2,6 @@ package integration_test import ( "os" - "os/exec" "path/filepath" "runtime" "strings" @@ -27,16 +26,7 @@ import ( func awsConfigEnv(t *testing.T) (env.Environ, string) { t.Helper() tmpHome := t.TempDir() - // Runs before t.TempDir() cleanup (LIFO order). The emulator runs as root - // inside the container, so files it writes into the volume are root-owned on - // Linux. Go's TempDir cleanup can't delete them, so we use a Docker container - // to remove them first. - t.Cleanup(func() { - volumeDir := filepath.Join(tmpHome, ".cache", "lstk", "volume") - if _, err := os.Stat(volumeDir); err == nil { - _ = exec.Command("docker", "run", "--rm", "-v", volumeDir+":/d", "alpine", "sh", "-c", "rm -rf /d/*").Run() - } - }) + scheduleVolumeCleanup(t, tmpHome) writeConfigFile(t, filepath.Join(tmpHome, ".config", "lstk", "config.toml")) e := env.With(env.AuthToken, env.Get(env.AuthToken)).WithHome(tmpHome) return e, tmpHome diff --git a/test/integration/completion_tip_test.go b/test/integration/completion_tip_test.go index dbf3d1e2..7f977e24 100644 --- a/test/integration/completion_tip_test.go +++ b/test/integration/completion_tip_test.go @@ -27,6 +27,9 @@ func firstRunHome(t *testing.T) (env.Environ, string) { t.Helper() tmpHome := t.TempDir() + // Every test built on this helper starts a real emulator, whose root-owned + // volume files would otherwise break TempDir cleanup on Linux. + scheduleVolumeCleanup(t, tmpHome) require.NoError(t, os.MkdirAll(filepath.Join(tmpHome, ".config"), 0755)) e := env.Environ(testEnvWithHome(tmpHome, tmpHome)).With(env.DisableEvents, "1") diff --git a/test/integration/main_test.go b/test/integration/main_test.go index f8b41d36..87a20945 100644 --- a/test/integration/main_test.go +++ b/test/integration/main_test.go @@ -179,6 +179,30 @@ const ( // startTestContainer starts the test container with no port bindings by default. // Pass hostPort to bind 4566/tcp to a specific host port (e.g. to test that lstk status // uses the actual bound port rather than the port from config). +// scheduleVolumeCleanup registers cleanup of the emulator volume under an +// isolated tmpHome. Call it right after t.TempDir(): t.Cleanup runs LIFO, so +// registering it later than TempDir's own cleanup is what makes it run first. +// +// The emulator runs as root inside the container, so on Linux the files it +// writes into the bind-mounted volume are root-owned and Go's TempDir cleanup +// cannot unlink them ("permission denied"), failing an otherwise passing test. +// Docker Desktop on macOS maps them to the calling user, which is why this only +// bites on Linux CI. Removing them from inside a container sidesteps the +// ownership problem entirely. +// +// Any test that both isolates HOME under t.TempDir() and starts a real emulator +// needs this. +func scheduleVolumeCleanup(t *testing.T, tmpHome string) { + t.Helper() + t.Cleanup(func() { + volumeDir := filepath.Join(tmpHome, ".cache", "lstk", "volume") + if _, err := os.Stat(volumeDir); err != nil { + return + } + _ = exec.Command("docker", "run", "--rm", "-v", volumeDir+":/d", "alpine", "sh", "-c", "rm -rf /d/*").Run() + }) +} + func startTestContainer(t *testing.T, ctx context.Context, hostPort ...string) { t.Helper() From 45d2c70e2d345cf5877fbcb499d572b3124fb7ef Mon Sep 17 00:00:00 2001 From: Joel Scheuner Date: Fri, 4 Sep 2026 15:07:44 +0200 Subject: [PATCH 3/4] Reword the completion tip to match the verb-first tip style Co-Authored-By: Claude --- internal/ui/run.go | 8 +++++--- test/integration/completion_tip_test.go | 9 ++++++--- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/internal/ui/run.go b/internal/ui/run.go index 8ff9a327..5d1a3e3c 100644 --- a/internal/ui/run.go +++ b/internal/ui/run.go @@ -71,9 +71,11 @@ type RunOptions struct { // The literal "> Tip: " prefix and SeveritySecondary match tipsForType in // internal/container/start.go, whose tip lands immediately above this one in // the post-start block — SeverityNote would render "> Note:" there instead and -// read as a second, unrelated idiom. -const completionTip = "> Tip: Tab completion is available using lstk completion [bash|zsh|fish|powershell] " + - "as documented at https://docs.localstack.cloud/aws/developer-tools/running-localstack/lstk/#shell-completions" +// read as a second, unrelated idiom. The wording follows the same shape for the +// same reason: an imperative verb, then a colon, then the command to run ("View +// emulator logs: lstk logs --follow"). Keep that form if this text changes. +const completionTip = "> Tip: Enable tab completion for your shell: lstk completion [bash|zsh|fish|powershell] " + + "See https://docs.localstack.cloud/aws/developer-tools/running-localstack/lstk/#shell-completions" func Run(parentCtx context.Context, runOpts RunOptions) error { ctx, cancel := context.WithCancel(parentCtx) diff --git a/test/integration/completion_tip_test.go b/test/integration/completion_tip_test.go index 7f977e24..d135c3c6 100644 --- a/test/integration/completion_tip_test.go +++ b/test/integration/completion_tip_test.go @@ -18,8 +18,8 @@ import ( // convention the neighbouring post-start tips use (tipsForType in // internal/container/start.go), so it is part of the observable behavior rather // than styling. -const completionTipText = "> Tip: Tab completion is available using lstk completion [bash|zsh|fish|powershell] " + - "as documented at https://docs.localstack.cloud/aws/developer-tools/running-localstack/lstk/#shell-completions" +const completionTipText = "> Tip: Enable tab completion for your shell: lstk completion [bash|zsh|fish|powershell] " + + "See https://docs.localstack.cloud/aws/developer-tools/running-localstack/lstk/#shell-completions" // firstRunHome returns an isolated home with no lstk config, so the run under // test is a first run (config.toml absent is what firstRun means). @@ -159,7 +159,10 @@ func TestFirstRunJSONEnvelopeHasNoCompletionTip(t *testing.T) { stdout, stderr, err := runLstk(t, testContext(t), "", e.With(env.APIEndpoint, mockServer.URL), "start", "--json") require.NoError(t, err, "lstk start --json failed: %s", stderr) - assert.NotContains(t, stdout, "Tab completion", "the tip must never reach machine-readable output") + // A distinctive substring rather than completionTipText: if the tip ever did + // leak into the envelope it would be inside a JSON string, where quoting or + // escaping could break an exact match and let the leak through unnoticed. + assert.NotContains(t, stdout, "tab completion", "the tip must never reach machine-readable output") envelope := decodeEnvelope(t, stdout) assert.Equal(t, "ok", envelope.Status) From 06c440ece0c9ca93a3169e23774eee7111f06487 Mon Sep 17 00:00:00 2001 From: Joel Scheuner Date: Tue, 8 Sep 2026 11:55:21 +0200 Subject: [PATCH 4/4] Condense the comments on this change and add a comment-length rule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses review feedback on #484: the comments were verbose even where the code was obvious. Trimmed 74 comment lines to 31, keeping only what a future reader would otherwise break — the MessageEvent/DeferredEvent constraint in Run, the LIFO ordering scheduleVolumeCleanup depends on, why --type still counts as a first run — and dropping the design narration around them. Also fixes startTestContainer's doc comment, which scheduleVolumeCleanup had been inserted in front of, silently reattaching it to the wrong function. CLAUDE.md gains a length budget in Code Style, and the "Maintaining This File" bullet no longer reads as an invitation to write essays in doc comments. Co-Authored-By: Claude --- CLAUDE.md | 7 +++-- cmd/root.go | 8 ++--- internal/ui/run.go | 41 ++++++++----------------- test/integration/completion_tip_test.go | 25 ++++++--------- test/integration/main_test.go | 26 ++++++---------- 5 files changed, 39 insertions(+), 68 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 960e6f2c..3ebfc08b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -203,6 +203,9 @@ The release job (`.github/workflows/ci.yml`) builds the npm packages with `gorel # Code Style - Don't add comments for self-explanatory code. Only comment when the "why" isn't obvious from the code itself. +- Keep comments short: 1–3 lines is the norm, ~6 is a ceiling that needs a real reason (a subtle invariant, upstream behaviour that will surprise the reader, a rejected alternative someone will otherwise re-attempt). Long comments are harder to read and go stale — they are a maintenance burden, not extra rigor. +- State each reason once, in the fewest words that carry it. Don't restate the code, the declaration's own name, or what the adjacent test already asserts, and don't narrate why the obvious path is safe. Where the design took some working out, record the constraint that came out of it rather than the path that led there — the constraint is what a future reader needs in order not to break it. +- If a declaration seems to need several unrelated paragraphs, that's a signal to split the code — not to grow the comment. - Do not remove comments added by someone else than yourself. - Errors returned by functions should always be checked unless in test files. - Terminology: in user-facing CLI/help/docs, prefer `emulator` over `container`/`runtime`; use `container`/`runtime` only for internal implementation details. @@ -220,7 +223,7 @@ The release job (`.github/workflows/ci.yml`) builds the npm packages with `gorel 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. -Only Homebrew installs get completion wired up automatically (`homebrew_casks.completions` in `.goreleaser.yaml`), so the first successful *interactive* start emits a one-line `> Tip:` pointing at `lstk completion [bash|zsh|fish|powershell]` and the docs section above (same prefix and severity as `tipsForType`, whose tip renders directly above it). It is deliberately just a pointer — lstk does not write to the user's shell config, and there is no `--write`/install flag (rationale on the `completionTip` const in `internal/ui/run.go`, which also explains why the trigger is the first run rather than install time). Adding a second such nudge means gating it the same way: `firstRun` only, interactive only, after the emulator is up. +Only Homebrew installs wire up completion automatically (`homebrew_casks.completions` in `.goreleaser.yaml`), so the first successful *interactive* start emits a one-line `> Tip:` pointing at `lstk completion [bash|zsh|fish|powershell]` and the docs. It is only a pointer — lstk never writes to the user's shell config, and there is no install flag (rationale on the `completionTip` const in `internal/ui/run.go`). A second such nudge must be gated the same way: `firstRun` only, interactive only, after the emulator is up. `lstk aws ` completes AWS services/operations/parameters by delegating to the AWS CLI's own `aws_completer` from a `ValidArgsFunction` on the `aws` command (DEVX-846) — `awscli.Complete` in `internal/awscli/complete.go`, wired in `cmd/aws.go`. Going through Cobra rather than registering `complete -C aws_completer lstk` is what makes it work in every shell `lstk completion` supports (the native registration is bash/zsh-only) and keeps the bash fallback above unchanged. `aws_completer` speaks bash's `complete -C` protocol: `COMP_LINE`/`COMP_POINT` in, candidates one-per-line out. Two constraints it imposes: the line must start with the literal word `aws` (the completer drops the first word before matching, so `lstk aws s3 l` returns nothing), and `COMP_POINT` is a **character** offset, not a byte one. Cobra never runs `PreRunE` on the `__complete` path, so completion stays offline — no config load, no Docker health check, no endpoint resolution — which matches the completer, which never contacts an endpoint. A missing or failing completer must return `ShellCompDirectiveDefault` and print nothing: any output on this path is read by the shell as a candidate. The Tab-press timeout is set by the caller in `cmd/aws.go` (`awsCompletionTimeout`), not inside `awscli.Complete` — an internal deadline made the unit test flaky on cold CI runs. That deadline only actually bounds a Tab press because `Complete` also sets `cmd.WaitDelay`: `exec.CommandContext` kills the completer on expiry but does not close a pipe the completer's own children still hold, so a completer leaving a grandchild on stdout (a wrapper script that forks instead of exec'ing — what `/bin/sh` does on Linux, where it is dash) made `cmd.Output()` block for the grandchild's full lifetime regardless of the context. Keep `WaitDelay` set on any similar short captured-output exec. `lstk az` needs the same treatment but a different protocol (argcomplete: `_ARGCOMPLETE=1`, output on fd 8). @@ -348,7 +351,7 @@ When making significant changes to the codebase (new commands, architectural cha **This is the only agent-instruction file in the repo.** Where each kind of detail should go instead: -- **Anything about a single declaration** (mechanism, rationale, invariants, why an obvious alternative was rejected, upstream/external behaviour it depends on) → a doc comment on that function, type, method, field, or constant. Negative statements work fine there too: anchor "there is deliberately no X" to the function where X would have gone. +- **Details about one function, type, method, field, or constant** → a doc comment on it, not here: how it works, why it's built that way, what has to stay true, why the obvious alternative doesn't work, and any outside behaviour it depends on. Same for what deliberately isn't there — put "there is no X, on purpose" on the function where X would have gone. Keep it concise (few lines). - **User-facing config reference** → `internal/config/default_config.toml`, which ships as the user's own commented config. - **User-facing command reference** → the command's Cobra `Short`/`Long` in `cmd/`, which is also what `lstk docs` renders. - **Design rationale and non-goals for in-flight work** → `openspec/changes//design.md`. diff --git a/cmd/root.go b/cmd/root.go index b421f6dc..fa4064c7 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -351,11 +351,9 @@ func startEmulator(ctx context.Context, rt runtime.Runtime, cfg *env.Env, tel *t logger.Info("could not resolve friendly config path: %v", err) } - // Captured before ApplyEmulatorType clears firstRun: `lstk start --type aws` - // on a fresh install is still the user's first run, and the completion tip - // should fire there too. firstRun itself is only cleared below to skip the - // emulator picker and the default-emulator notice, both of which --type has - // already answered. + // Captured before ApplyEmulatorType clears firstRun to skip the picker and + // default-emulator notice. `lstk start --type aws` on a fresh install is + // still a first run, so the completion tip must fire there too. wasFirstRun := firstRun // Apply the --type flag before resolving snapshot and start options so diff --git a/internal/ui/run.go b/internal/ui/run.go index 5d1a3e3c..8cdd763d 100644 --- a/internal/ui/run.go +++ b/internal/ui/run.go @@ -41,39 +41,22 @@ type RunOptions struct { // auto-load a configured snapshot). It is skipped when the emulator was // already running. PostStart func(ctx context.Context, sink output.Sink) error - // CompletionTip points the user at the documented shell-completion setup - // once the emulator is up. The caller decides when it applies (the first - // run, i.e. config.toml was absent) so this package holds no policy; see - // completionTip for why the first run is the trigger. + // CompletionTip shows completionTip once the emulator is up. The caller + // decides when it applies (first run), so this package holds no policy. CompletionTip bool } -// completionTip nudges users toward the completion scripts lstk already ships. -// Only Homebrew installs get them set up automatically (`.goreleaser.yaml`'s -// homebrew_casks.completions), so npm and GitHub-release users have to find the -// docs themselves and most never do. +// completionTip points at the completion scripts lstk ships. Only Homebrew +// installs wire them up automatically (homebrew_casks.completions in +// .goreleaser.yaml), so npm and binary users never find them. First run is the +// trigger because no install path offers a usable hook (generated npm +// package.json, no hook at all for binaries) and it needs no new persisted +// state: config.toml was absent, and that same run creates it. // -// The trigger is the first run rather than install time, because no install -// path offers a usable hook: the npm package.json is generated by -// goreleaser-npm-publisher, a postinstall that edits a shell rc is hostile and -// is skipped under --ignore-scripts, and binary installs have no hook at all. -// The first run needs no new persisted state either — it means "config.toml was -// absent", and that same path creates the config, so the tip cannot repeat. -// -// It is emitted after the emulator is up, not before, so it does not become a -// third interruption ahead of the emulator picker and the update prompt. It is -// a plain inline MessageEvent rather than a DeferredEvent: Run does not render -// DeferredOutput (only runWithTUI does), so a deferred event would be silently -// dropped on this path. Emitting it immediately before runDoneMsg is safe -// because both sends cross the same ordered channel, and buffered lines are -// always flushed before the app quits. -// -// The literal "> Tip: " prefix and SeveritySecondary match tipsForType in -// internal/container/start.go, whose tip lands immediately above this one in -// the post-start block — SeverityNote would render "> Note:" there instead and -// read as a second, unrelated idiom. The wording follows the same shape for the -// same reason: an imperative verb, then a colon, then the command to run ("View -// emulator logs: lstk logs --follow"). Keep that form if this text changes. +// Must stay a plain MessageEvent, not a DeferredEvent — Run does not render +// DeferredOutput (only runWithTUI does), so a deferred event would be dropped. +// The "> Tip: " prefix, SeveritySecondary, and verb-colon-command wording match +// tipsForType (internal/container/start.go), whose tip renders right above it. const completionTip = "> Tip: Enable tab completion for your shell: lstk completion [bash|zsh|fish|powershell] " + "See https://docs.localstack.cloud/aws/developer-tools/running-localstack/lstk/#shell-completions" diff --git a/test/integration/completion_tip_test.go b/test/integration/completion_tip_test.go index d135c3c6..efca0c76 100644 --- a/test/integration/completion_tip_test.go +++ b/test/integration/completion_tip_test.go @@ -12,12 +12,9 @@ import ( "github.com/stretchr/testify/require" ) -// completionTipText is the user-visible tip lstk shows once, after the first -// successful interactive start, pointing at the documented shell-completion -// setup. Asserted verbatim, including the "> Tip: " prefix: that prefix is the -// convention the neighbouring post-start tips use (tipsForType in -// internal/container/start.go), so it is part of the observable behavior rather -// than styling. +// completionTipText is asserted verbatim, "> Tip: " prefix included: that +// prefix is the convention the neighbouring post-start tips use (tipsForType in +// internal/container/start.go), so it is observable behavior, not styling. const completionTipText = "> Tip: Enable tab completion for your shell: lstk completion [bash|zsh|fish|powershell] " + "See https://docs.localstack.cloud/aws/developer-tools/running-localstack/lstk/#shell-completions" @@ -27,8 +24,6 @@ func firstRunHome(t *testing.T) (env.Environ, string) { t.Helper() tmpHome := t.TempDir() - // Every test built on this helper starts a real emulator, whose root-owned - // volume files would otherwise break TempDir cleanup on Linux. scheduleVolumeCleanup(t, tmpHome) require.NoError(t, os.MkdirAll(filepath.Join(tmpHome, ".config"), 0755)) e := env.Environ(testEnvWithHome(tmpHome, tmpHome)).With(env.DisableEvents, "1") @@ -54,11 +49,11 @@ func TestFirstRunShowsCompletionTip(t *testing.T) { p := startLstkInPTY(t, testContext(t), e.With(env.APIEndpoint, mockServer.URL), "start") - // First run shows the emulator picker; accept the highlighted default (AWS). + // First run shows the emulator picker; accept the default (AWS). p.waitForOutput("Which emulator would you like to use?", "emulator selection prompt should appear on first run") p.write("\r") - // Post-start setup asks about the AWS CLI profile in the isolated home; decline it. + // Post-start setup asks about the AWS CLI profile; decline it. p.waitForOutputTimeout(awsSetupPrompt, 2*time.Minute, "container should become ready") p.write("n") @@ -68,9 +63,8 @@ func TestFirstRunShowsCompletionTip(t *testing.T) { assert.Contains(t, out, completionTipText, "first successful interactive start should point at shell completion setup") } -// --type is the non-interactive answer to the first-run emulator picker, so it -// suppresses the picker — but the run is still the user's first, and the tip -// must survive that. +// --type answers the first-run picker, so it suppresses it — but the run is +// still a first run and the tip must survive that. func TestFirstRunWithEmulatorTypeFlagShowsCompletionTip(t *testing.T) { requireDocker(t) _ = env.Require(t, env.AuthToken) @@ -159,9 +153,8 @@ func TestFirstRunJSONEnvelopeHasNoCompletionTip(t *testing.T) { stdout, stderr, err := runLstk(t, testContext(t), "", e.With(env.APIEndpoint, mockServer.URL), "start", "--json") require.NoError(t, err, "lstk start --json failed: %s", stderr) - // A distinctive substring rather than completionTipText: if the tip ever did - // leak into the envelope it would be inside a JSON string, where quoting or - // escaping could break an exact match and let the leak through unnoticed. + // A distinctive substring, not completionTipText: inside a JSON string, + // escaping could break an exact match and hide the leak. assert.NotContains(t, stdout, "tab completion", "the tip must never reach machine-readable output") envelope := decodeEnvelope(t, stdout) diff --git a/test/integration/main_test.go b/test/integration/main_test.go index 87a20945..ba92d5ce 100644 --- a/test/integration/main_test.go +++ b/test/integration/main_test.go @@ -176,22 +176,13 @@ const ( testImage = "alpine:latest" ) -// startTestContainer starts the test container with no port bindings by default. -// Pass hostPort to bind 4566/tcp to a specific host port (e.g. to test that lstk status -// uses the actual bound port rather than the port from config). -// scheduleVolumeCleanup registers cleanup of the emulator volume under an -// isolated tmpHome. Call it right after t.TempDir(): t.Cleanup runs LIFO, so -// registering it later than TempDir's own cleanup is what makes it run first. -// -// The emulator runs as root inside the container, so on Linux the files it -// writes into the bind-mounted volume are root-owned and Go's TempDir cleanup -// cannot unlink them ("permission denied"), failing an otherwise passing test. -// Docker Desktop on macOS maps them to the calling user, which is why this only -// bites on Linux CI. Removing them from inside a container sidesteps the -// ownership problem entirely. -// -// Any test that both isolates HOME under t.TempDir() and starts a real emulator -// needs this. +// scheduleVolumeCleanup removes the emulator volume under an isolated tmpHome. +// Call it right after t.TempDir(): t.Cleanup runs LIFO, so registering it later +// is what makes it run before TempDir's own cleanup. The emulator runs as root +// in the container, so on Linux its volume files are root-owned and TempDir +// cleanup cannot unlink them (Docker Desktop on macOS maps them to the calling +// user); deleting them from inside a container sidesteps that. Needed by any +// test that isolates HOME under t.TempDir() and starts a real emulator. func scheduleVolumeCleanup(t *testing.T, tmpHome string) { t.Helper() t.Cleanup(func() { @@ -203,6 +194,9 @@ func scheduleVolumeCleanup(t *testing.T, tmpHome string) { }) } +// startTestContainer starts the test container with no port bindings by default. +// Pass hostPort to bind 4566/tcp to a specific host port (e.g. to test that lstk status +// uses the actual bound port rather than the port from config). func startTestContainer(t *testing.T, ctx context.Context, hostPort ...string) { t.Helper()