experiment(cli): evaluate AgentENV-owned completion hook - #164
Conversation
Captures the sandbox-ID-only design, its short-timeout failure behavior, shell-adapter boundary, and verification plan before implementation begins. Constraint: PR kvcache-ai#89 provides static completion but no stable dynamic hook\nRejected: Use clap_complete unstable-dynamic API | maintainer identified the surface as unstable\nConfidence: high\nScope-risk: narrow\nDirective: Keep dynamic lookup silent and separate from normal CLI timeouts\nTested: Design self-review and git diff --check\nNot-tested: CLI behavior; implementation has not started
Evaluate a stable AgentENV-owned completion protocol alongside the static shell generator, using a hidden command for dynamic sandbox IDs. Constraint: The experiment must preserve the existing clap-generated scripts and keep completion failures silent Rejected: Depend on clap_complete unstable-dynamic | leaves the protocol and shell adapter contract outside AgentENV Confidence: medium Scope-risk: moderate Directive: Treat __complete as an internal protocol and coordinate any future changes with all three shell adapters Tested: cargo fmt --all; cargo check -p aenv --bin aenv; focused completion tests (15 passed); generated Bash script; no-credential hook invocation Not-tested: Full workspace test suite and live API-backed candidate lookup
|
🔍 OpenCodeReview found 5 issue(s) in this PR.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| "pause" | "exec" | "upload" | "download" | "timeout" if index == 2 => { | ||
| SandboxStateFilter::Running | ||
| } | ||
| "resume" if index == 2 => SandboxStateFilter::Paused, |
There was a problem hiding this comment.
[bug · medium]
The fixed cursor index assumes the sandbox ID immediately follows the subcommand, but Clap permits options before positional arguments. For example, the documented aenv upload --user app <TAB>, aenv download --force <TAB>, and aenv resume --timeout 300 <TAB> forms reach indices greater than 2 and receive no dynamic candidates. Determine the active positional after skipping each command's options (including option values), rather than keying solely on index == 2.
| "snapshot" | ||
| if words.get(2).map(String::as_str) == Some("list") | ||
| && index > 3 | ||
| && words[..index].last().map(String::as_str) == Some("--sandbox-id") => |
There was a problem hiding this comment.
[bug · medium]
This excludes the declared visible alias snapshot ls, even though the command's own examples use aenv snapshot ls --sandbox-id <sandbox-id>. Accept both list and ls here so the documented form receives sandbox completion.
| && words[..index].last().map(String::as_str) == Some("--sandbox-id") => | ||
| { | ||
| SandboxStateFilter::Active | ||
| } |
There was a problem hiding this comment.
[bug · medium]
Only the separated --sandbox-id <value> form is recognized. Clap also accepts --sandbox-id=<value>, for which the current token contains the option and prefix and this branch returns None (notably in shells that preserve = in the word). Handle the attached form and emit candidates with the --sandbox-id= prefix so completion does not replace the entire option token with a bare ID.
| fi | ||
| _aenv "$@" | ||
| } | ||
| complete -o nospace -o bashdefault -o nosort -F _aenv_dynamic aenv |
There was a problem hiding this comment.
[bug · medium]
-o nospace applies to every result from this wrapper, including the fallback _aenv static completions. Consequently Bash no longer appends a space after completing ordinary subcommands, options, or their values, and the next token can be concatenated to the completion. Preserve the options from the generated _aenv registration (or conditionally manage spacing only for dynamic results) instead of globally enabling nospace.
| set -l words (commandline -opc) | ||
| set -a words (commandline -ct) | ||
| command aenv __complete --index (math (count $words) - 1) -- $words 2>/dev/null |
There was a problem hiding this comment.
[bug · medium]
When the cursor is immediately after a space, commandline -ct is empty and its unquoted command substitution appends no array element. The calculated index then points at the preceding subcommand (for example pause) rather than an empty sandbox argument, so Fish offers no IDs until the user types a prefix. Store the current token separately and append it as a quoted value so the empty element is preserved.
1 participant
Summary
This is the second parallel experiment for #37. It keeps the static completion generator and appends Bash/Zsh/Fish adapters that call an AgentENV-owned internal protocol:
aenv __complete.What it adds
aenv __complete --index ... -- <shell words...>entry point.Trade-off
The protocol is owned by AgentENV and does not depend on
clap_completeunstable-dynamic, but AgentENV must maintain cursor parsing plus three shell adapters. Clap still includes the hidden hook in generated command metadata, which is a small maintenance detail to discuss. The companion PR evaluates the upstream unstable dynamic engine. These are alternatives; please review and choose one direction rather than merging both.Verification
cargo fmt --allcargo check -p aenv --bin aenv