Add shell completion for lstk aws - #424
Draft
skyrpex wants to merge 2 commits into
Draft
Conversation
Co-Authored-By: Claude <noreply@anthropic.com>
exec.CommandContext kills the completer on deadline but does not close a pipe its own children still hold, so a completer leaving a grandchild on stdout blocked cmd.Output() for that grandchild's full lifetime. On Linux /bin/sh is dash, which forks rather than execs, so the deadline test hung for 30s instead of 100ms. Co-Authored-By: Claude <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
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.
Motivation
awsandawslocalboth have rich Tab completion;lstk awshad none. Reported in Slack while checkinglstk awsfor parity withawslocal.Changes
lstk aws <TAB>now completes AWS services, operations and parameters by delegating to the AWS CLI's ownaws_completer.internal/awscli/complete.go:CompleterPath()(PATH, falling back to a sibling of the symlink-resolvedawsbinary) andComplete(ctx, args, toComplete), which speaks the sameCOMP_LINE/COMP_POINTprotocol bash'scomplete -Cuses.ValidArgsFunctionon theawscommand incmd/aws.go, with the Tab-press timeout owned at the command boundary.lstk aws --helpmentions that completion comes withlstk completion <shell>.Delegating through Cobra rather than registering
complete -C aws_completer lstkis what makes this work in every shelllstk completionsupports — the native AWS registration is bash/zsh-only — and it needs no additions to the self-contained bash 3.2 fallback from DEVX-950.Three protocol details the implementation has to respect, all documented in comments:
COMP_LINEmust start with the literal wordaws: the completer drops the first word before matching, so handing itlstk aws s3 lreturns nothing at all.COMP_POINTis a character offset, not a byte offset.Cobra does not run
PreRunEon the__completepath, so completion stays fully offline: no config load, no Docker health check, no endpoint resolution. That matchesaws_completer, which completes commands and parameters only and never contacts an endpoint. A missing or failing completer returnsShellCompDirectiveDefaultand prints nothing — any output on this path would be read by the shell as a candidate.On
cmd.WaitDelayWorth a look during review, since the reason isn't visible from the call site.
exec.CommandContextkills the completer when the deadline expires, but it does not close a pipe the completer's own children still hold — socmd.Output()keeps blocking until every holder of stdout exits. A completer that forks instead of exec'ing its payload would therefore hang a Tab press for as long as that grandchild lives, deadline or no deadline. The pip-installed v1aws_completeris a script, so this is not hypothetical.This is platform-dependent, which is how it got past a first green local run: on macOS
/bin/shis bash 3.2 and exec's the payload (killing the child is enough), while on Linux it is dash, which forks. The deadline test consequently blocked for 30s against a 100ms deadline on CI and 100ms on macOS.cmd.WaitDelaycaps the post-deadline drain, then closes the pipes.The test's fake completer now forks explicitly (
sleep N & wait) rather than relying on which shell/bin/shhappens to be, so it exercises the grandchild case on every platform. Verified in agolang:1.25container that it fails (30.0s) with only theWaitDelayline removed and passes (0.21s) with it — the test is not vacuous.Review
Human review advised — new user-facing behavior, and the shell-completion protocol has failure modes (stray stdout corrupting candidates) that don't surface as test failures.
Tests
internal/awscli/complete_test.go:COMP_LINEconstruction, output parsing, completer discovery (PATH andaws-sibling fallback), and the deadline bound described above.test/integration/aws_completion_test.go, including one that drives the real generated bash script the way a Tab press does.runBashCompletionDrivergrew a variadicextraPathso a stand-inaws_completeris visible to thelstk __completesubprocess.make test(1266 pass),make lint(0 issues), and all 23TestAWSCommandintegration tests pass.Manually verified against the real
aws_completer(aws-cli 2.35.11) in stock macOS bash 3.2. ~140 ms per Tab press.Manually testing this
Build first:
Confirm the AWS CLI's completer is installed — without it this feature is a deliberate no-op:
Ask for completions directly. This is exactly what every shell's completion script calls, so it needs no shell setup:
Each prints the candidates, then a
:4directive line.Now the real thing. In bash:
In zsh:
Then type these and press Tab:
Completion must not need Docker or a running emulator — this still returns
ls:And on a machine without
aws_completerit must degrade silently, printing only:0(hand the word back to the shell's file completion) and never an error:Todo
lstk azneeds the same treatment, but a different protocol (argcomplete:_ARGCOMPLETE=1, output on fd 8).Closes DEVX-846