From 137cef758508de4b630de6d2351f86f60cd36d7c Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Wed, 9 Sep 2026 13:12:39 +0000 Subject: [PATCH 1/4] fix: a flag before the workspace spec completed nothing `aid --codex owner/repo` offered no completions at all, and neither did `dl --devcontainer robot owner/repo`. The completion script found the spec by counting words from the command -- word two for the spec, word three for a verb -- and neither grammar works that way. aid reads its leading flags and calls the first word that is not one the spec (`parse_aid_args`), and dl is clap, which puts options anywhere among the positional words. The script now scans the words before the cursor and counts the positional ones, stepping over a value option and its value and stopping at `--`, so the spec is wherever it actually lands. That needs one distinction the old code could not make: a flag that *is* the command (`--ls`, no workspace follows) against one that modifies a launch (`--rm`, a spec is still to come). The old guard ended completion on any leading `--`, so closing the gap for `--rm` would have opened `dl --ls ` up to offering workspaces to a command that takes none. The new table is clap's own `what` group, whose members are mutually exclusive because each one is the whole command, and `completion_tables.rs` diffs the two -- both halves, plus aid's. Claude-Session: https://claude.ai/code/session_01GxeUVm3R579Tqai6GwoEmb --- rust/devlaunch-core/completions/dl.bash | 98 ++++++++++++++++++++----- rust/dl/tests/completion_tables.rs | 93 ++++++++++++++++++++++- test/test_bash_completion.py | 66 +++++++++++++++++ 3 files changed, 235 insertions(+), 22 deletions(-) diff --git a/rust/devlaunch-core/completions/dl.bash b/rust/devlaunch-core/completions/dl.bash index db7219d1..b8f7416c 100644 --- a/rust/devlaunch-core/completions/dl.bash +++ b/rust/devlaunch-core/completions/dl.bash @@ -54,13 +54,18 @@ _dl_completion() { cmd="${words[0]##*/}" fi - # Global command options (only valid as first arg). + # The options offered where a workspace spec goes. # # Every user-facing flag dl's argument grammar declares, and a test diffs the - # two: `dl/tests/completion_tables.rs`. Four are deliberately absent — - # --json, --size, --yes and --force modify a line that already named a - # command, so none of them is ever the first word — and that test names them - # with the reason. Anything added below has to be added there too. + # two: `dl/tests/completion_tables.rs`. Five are deliberately absent — + # --json, --size, --yes, --force and --force-worktrees modify a command that + # is already on the line, and a line that named one is a line this function + # stops completing — and that test names them with the reason. Anything added + # below has to be added there too. + # + # "Where the spec goes" is not "the second word": a modifying flag can come + # first, so these are offered after one too (`dl --rm --`). Which word + # that is, is the scan further down. # # The retired spellings (--stop, --autorm) are absent by rule rather than by # hand: the grammar marks them `hide = true`, and the test drops every hidden @@ -80,6 +85,23 @@ _dl_completion() { # Options that take a value; a variant name, a profile name or a path follows. local value_opts="--devcontainer --claude-profile" + # The flags that are a command in themselves: they say what to do, and no + # workspace follows one. Every flag not here modifies a launch that still + # needs a spec, which is the distinction the scan below turns on -- `dl --ls` + # is finished and `dl --devcontainer robot` has not started. + # + # Derived rather than judged: these are exactly the flags dl declares in + # clap's `what` group, whose members are mutually exclusive because each one + # *is* the command, plus the help and version clap generates. + # `dl/tests/completion_tables.rs` diffs the two. + local command_opts="--ls --install --refresh --prune --reconcile --purge --herdr-shell --claude-profiles --help -h --version" + if [[ "$cmd" == aid ]]; then + # The three aid answers itself, before it rewrites anything into a dl + # line. Every other aid flag is read ahead of the spec and passed + # through, so the spec is still to come. + command_opts="--help -h --version" + fi + # After --claude-profile, offer the profile directories that exist. Read off the # disk rather than out of the completion cache, deliberately: profiles are # created by hand and rarely, the cache is rebuilt by commands that change @@ -157,8 +179,54 @@ _dl_completion() { source "$cache_file" fi - # First argument: global flags, workspaces, repos, owners, or paths - if [[ ${word_count} -eq 2 ]]; then + # Which positional slot the word being completed sits in: the spec is the + # first, a verb the second. + # + # Read off the words before it rather than counted from the command, because + # a flag can precede the spec in both grammars and counting cannot see that. + # `aid --codex owner/repo` is the line that reported this: the spec was + # offered at word two alone, so every agent-flag line completed nothing, and + # so did `dl --devcontainer robot owner/repo`. dl's rule is clap's -- options + # sit anywhere among the positional words -- and aid's is `parse_aid_args`, + # which reads the leading flags and calls the first word that is not one the + # spec. + # + # The words strictly before `cur` are indices 1 through word_count-2, which + # holds whether or not the line ends in a space: the trailing-space branch + # above incremented word_count without appending to `words`. + local position=0 named_a_command=0 scan=1 scanned + while (( scan <= word_count - 2 )); do + scanned="${words[scan]}" + if [[ "$scanned" == "--" ]]; then + # Everything past it is the command run inside the workspace, which + # is the user's shell to complete and not ours. + return 0 + fi + if [[ " ${value_opts} " == *" ${scanned} "* ]]; then + # Its value is not a positional word, so step over the pair. + (( scan += 2 )) + continue + fi + if [[ "$scanned" == -* ]]; then + if [[ " ${command_opts} " == *" ${scanned} "* ]]; then + named_a_command=1 + fi + (( scan++ )) + continue + fi + (( position++ )) + (( scan++ )) + done + + # A line that already named a command takes neither a workspace nor a verb. + # This is what used to be a guard on the first word starting with `--`, which + # could not tell `dl --ls` from `dl --rm`. + if (( named_a_command )); then + return 0 + fi + + # The spec's position: flags, workspaces, repos, owners, or paths. + if (( position == 0 )); then # Global flags if [[ ${cur} == -* ]]; then COMPREPLY=( $(compgen -W "${global_opts}" -- ${cur}) ) @@ -247,19 +315,9 @@ _dl_completion() { return 0 fi - # Second argument (after workspace): subcommands. Everything after an aid - # workspace is the prompt, so there is nothing to offer there. - if [[ ${word_count} -eq 3 && "$cmd" != aid ]]; then - # Don't complete after global flags - # Extract the first argument (word after "dl") from the words array - local first="" - if (( ${#words[@]} > 1 )); then - first="${words[1]}" - fi - if [[ "$first" == --* ]]; then - return 0 - fi - + # The verb's position, after the spec. Everything after an aid workspace is + # the prompt, so there is nothing to offer there. + if (( position == 1 )) && [[ "$cmd" != aid ]]; then COMPREPLY=( $(compgen -W "${ws_cmds}" -- ${cur}) ) return 0 fi diff --git a/rust/dl/tests/completion_tables.rs b/rust/dl/tests/completion_tables.rs index df917d2f..022a279f 100644 --- a/rust/dl/tests/completion_tables.rs +++ b/rust/dl/tests/completion_tables.rs @@ -126,6 +126,10 @@ struct Flag { /// A value follows this flag as a separate word, which the script has to know /// so it can complete the value instead of another flag. takes_value: bool, + /// `group = "what"`: the flag *is* the command, so no workspace follows it. + /// The group exists because its members are mutually exclusive for exactly + /// that reason, which is what makes it the original this fact is copied from. + names_a_command: bool, } /// Every flag the `Cli` struct declares, in declaration order. @@ -172,6 +176,7 @@ fn grammar_flags(grammar: &str) -> Vec { long: format!("--{long}"), hidden: parts.contains(&"hide = true"), takes_value: ty.trim().trim_end_matches(',') != "bool", + names_a_command: parts.contains(&"group = \"what\""), }); } assert!( @@ -284,8 +289,13 @@ fn aid_flag_list(rewrite: &str, name: &str) -> BTreeSet { /// writes it here — which is the decision this guard exists to force. The common /// thread is position: the completion offers this table where a *command* goes, and /// none of these is one. They modify a line that already named one, and the script -/// offers nothing in that position at all (a first word starting with `--` ends -/// completion) — a gap worth closing, but a different change than this. +/// stops completing a line that has named one, so there is nowhere to offer them. +/// +/// A leading `--` no longer ends completion by itself — the script tells a flag +/// that is the command from one that modifies a launch, which is what +/// [`the_flags_that_end_a_line_are_the_grammars_own_command_group`] pins — so a +/// modifier reaches this table now (`dl --rm --`). These five still do not, +/// because each modifies a *command* flag and the script offers nothing after one. const NOT_OFFERED_FIRST: [(&str, &str); 5] = [ ( "--json", @@ -415,6 +425,63 @@ fn the_flags_a_value_follows_are_the_grammars_value_taking_flags() { ); } +#[test] +fn the_flags_that_end_a_line_are_the_grammars_own_command_group() { + // The script has to tell a flag that *is* the command from one that modifies a + // launch, because that is what says whether a workspace spec is still to come: + // `dl --ls` is finished and `dl --devcontainer robot` has not started. Get it + // wrong in one direction and `dl --rm ` offers nothing; wrong in the other + // and `dl --ls ` offers every workspace to a command that takes none. + // + // clap's `what` group is the original, and it is one rather than a coincidence: + // its members are mutually exclusive precisely because each one is the whole + // command. Hidden flags are dropped for the same reason as everywhere else here + // -- a spelling `--help` does not show is not one the script offers -- and the + // two retired ones are deliberately outside the group anyway. + let script = completion_script(); + let grammar = argument_grammar(); + + let mut expected: BTreeSet = grammar_flags(&grammar) + .iter() + .filter(|flag| flag.names_a_command && !flag.hidden) + .map(|flag| flag.long.clone()) + .collect(); + // clap generates these, and both end a line: `dl --help owner/repo` prints the + // help and opens nothing. + expected.insert("--help".to_owned()); + expected.insert("-h".to_owned()); + + assert_eq!( + assigned(&script, "local command_opts="), + expected, + "the completion script's command flags have drifted from the grammar's \ + `what` group" + ); + assert!( + assigned(&script, "local command_opts=").is_subset(&dl_first_argument_flags(&script)), + "a flag that ends a line is a flag that can be tabbed to in the first place" + ); +} + +#[test] +fn a_flag_beside_a_workspace_is_never_one_that_ends_the_line() { + // The two tables read the same word differently, so a word in both would be a + // contradiction: `ws_cmds` offers `--rm` *after* a spec, which only makes sense + // for a flag the scan lets a spec follow. + let script = completion_script(); + + let ends_the_line = assigned(&script, "local command_opts="); + let (_, beside) = dl_workspace_words(&script); + + for flag in &beside { + assert!( + !ends_the_line.contains(flag), + "{flag} is offered beside a workspace and also listed as ending the \ + line, and it cannot be both" + ); + } +} + /// Flags `aid` offers for a first argument beyond one per agent, and where each /// comes from. /// @@ -455,3 +522,25 @@ fn aid_offers_one_flag_per_agent_it_can_start() { "the completion script's aid flags have drifted from aid's agent table" ); } + +#[test] +fn the_only_aid_flags_that_end_a_line_are_the_three_aid_answers_itself() { + // aid's half of `the_flags_that_end_a_line_are_the_grammars_own_command_group`. + // There is no group to derive it from -- aid's grammar is not a clap grammar -- + // but there is already a hand-written table of the flags aid answers before it + // rewrites anything, and those are exactly the flags no workspace follows. Every + // other aid flag, agent flags and unknown pass-throughs alike, is read ahead of + // the spec, so the spec is still to come. + let script = completion_script(); + + let expected: BTreeSet = AID_FLAGS_BESIDE_THE_AGENTS + .iter() + .map(|(flag, _)| (*flag).to_owned()) + .collect(); + + assert_eq!( + assigned(&script, "command_opts="), + expected, + "aid's command flags have drifted from the flags aid answers itself" + ); +} diff --git a/test/test_bash_completion.py b/test/test_bash_completion.py index 1289bcd8..bdba7bff 100644 --- a/test/test_bash_completion.py +++ b/test/test_bash_completion.py @@ -314,6 +314,72 @@ def test_no_subcommand_completion_after_an_aid_workspace(self): for cmd in ["up", "stop", "rm", "code", "restart", "recreate", "reset"]: assert cmd not in completions + def test_an_agent_flag_does_not_move_the_workspace_spec(self): + """`aid --codex ` completes its spec exactly as `aid ` does. + + aid reads its leading flags and calls the first word that is not one the + workspace spec (`parse_aid_args`), so an agent flag does not occupy the + spec's position. The script counted words from the command instead and + offered a spec at word two alone, which made every `aid --codex`, + `aid --claude` and `aid --gemini` line complete nothing at all. + """ + bare = self.run_completion("aid my-") + assert bare, "the line the flag is added to completes something to begin with" + for flag in ["--claude", "--codex", "--gemini"]: + assert self.run_completion(f"aid {flag} my-") == bare, ( + f"`aid {flag} my-` completes differently from `aid my-`, but the " + "flag is read before the spec and does not stand in its place" + ) + + def test_a_value_option_and_its_value_do_not_move_the_workspace_spec(self): + """`dl --devcontainer robot ` completes its spec too. + + The same defect as the agent flags, two words wide instead of one: + clap puts options anywhere among the positional words, so the variant + name is not the workspace and the workspace is not word two. + """ + bare = self.run_completion("dl my-") + assert bare + assert self.run_completion("dl --devcontainer robot my-") == bare + assert self.run_completion("dl --claude-profile work my-") == bare + + def test_a_leading_modifier_leaves_the_verb_where_it_was(self): + """`dl --rm `: a flag ahead of the spec shifts nothing. + + `--rm` modifies a launch rather than being one, so the spec is still to + come after it and a verb after that. + """ + assert self.run_completion("dl --rm my-workspace ") == self.run_completion( + "dl my-workspace " + ) + + def test_a_command_flag_ends_the_line_whatever_follows_it(self): + """A flag that *is* the command takes neither a workspace nor a verb. + + The distinction the scan turns on, and the half that is easy to lose + when the other half starts working: `--devcontainer` has not started a + line and `--ls` has finished one, so only the second stops here. + """ + for line in ["dl --ls ", "dl --purge ", "dl --prune ", "dl --version ", "dl --ls my-"]: + assert self.run_completion(line) == [], ( + f"`{line}` completes something, but the flag on it names the whole " + "command and no workspace follows one" + ) + + def test_nothing_is_completed_past_the_double_dash(self): + """After `--` the words are the command run inside the workspace.""" + for line in ["dl my-workspace -- ", "dl my-workspace -- ls my-", "dl --rm my-ws -- "]: + assert self.run_completion(line) == [] + + def test_an_aid_prompt_is_never_a_verb_however_the_line_began(self): + """aid's prompt starts after the spec, leading flags or not.""" + for line in ["aid my-workspace ", "aid --codex my-workspace ", "aid --codex my-ws stop "]: + for verb in ["up", "stop", "rm", "code", "restart", "recreate", "reset"]: + assert verb not in self.run_completion(line), ( + f"`{line}` offers the verb {verb}, but everything after an aid " + "workspace is prompt text" + ) + def test_completion_partial_workspace_match(self): """Test partial matching of workspace names.""" # Complete after typing "dl test" From a005d25f411e2a9df4794babb6b213c0b0917cbf Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Wed, 9 Sep 2026 13:24:06 +0000 Subject: [PATCH 2/4] fix: ten flags completed a workspace onto a line dl refuses The scan added in the previous commit put the default arm on the wrong side. It listed the flags that *end* a line and treated everything else as a launch modifier a spec still follows, and the flags nobody thinks to list are all on the ending side: dl --repos my-workspace "--repos takes no workspace" dl --update-cache my-workspace the same dl --json my-workspace clap error, --json requires --ls dl --yes my-workspace "--yes means nothing for a workspace command" dl --force my-workspace "Unknown workspace '--force'" dl --stop / --autorm retired, refused by name Ten in all: the command group's hidden members, which the `!hidden` filter dropped from the table, the five that need something already on the line, and the two retired spellings. Every one of them completed `[]` before, and the fix offered a workspace name for each -- tab to a name and get refused for something you never typed, which is the bar `test_the_completion_offers_only_names_a_launch_accepts` already holds the profile names to. The table is inverted: `spec_follows` lists the flags a spec may follow and every other flag ends the line, so the default arm is the refusal. That is three flags for dl rather than sixteen, still derived -- every flag, minus clap's `what` group, minus the hidden ones, minus `NOT_OFFERED_FIRST` -- and it also covers an unknown flag, which the listed-endings form got wrong for free. Two more, both caught by mutation: - `test_a_leading_modifier_leaves_the_verb_where_it_was` compared two empty lists and passed with the verb branch mutated to `position == 99`. Anchored, along with the `--` test beside it. - The sameness the report asked for includes whether the cursor is left against the `/`, which COMPREPLY does not show. The agent-flag test now diffs `compopt` too, across five spec shapes. `aid_flag_list` read one of `REMOTE_CONTROL_FLAGS`' two spellings, because the first entry is a named const rather than a literal, and compared the short list against the script -- the silent pass this file exists to prevent. It resolves the const and asserts one flag per entry. Claude-Session: https://claude.ai/code/session_01GxeUVm3R579Tqai6GwoEmb --- rust/devlaunch-core/completions/dl.bash | 82 ++++++---- rust/dl/tests/completion_tables.rs | 205 +++++++++++++++++------- test/test_bash_completion.py | 101 ++++++++++-- 3 files changed, 288 insertions(+), 100 deletions(-) diff --git a/rust/devlaunch-core/completions/dl.bash b/rust/devlaunch-core/completions/dl.bash index b8f7416c..2797ad0a 100644 --- a/rust/devlaunch-core/completions/dl.bash +++ b/rust/devlaunch-core/completions/dl.bash @@ -58,10 +58,9 @@ _dl_completion() { # # Every user-facing flag dl's argument grammar declares, and a test diffs the # two: `dl/tests/completion_tables.rs`. Five are deliberately absent — - # --json, --size, --yes, --force and --force-worktrees modify a command that - # is already on the line, and a line that named one is a line this function - # stops completing — and that test names them with the reason. Anything added - # below has to be added there too. + # --json, --size, --yes, --force and --force-worktrees each need something + # already on the line, so none is ever the first word, and that test names + # them with the reason. Anything added below has to be added there too. # # "Where the spec goes" is not "the second word": a modifying flag can come # first, so these are offered after one too (`dl --rm --`). Which word @@ -85,21 +84,31 @@ _dl_completion() { # Options that take a value; a variant name, a profile name or a path follows. local value_opts="--devcontainer --claude-profile" - # The flags that are a command in themselves: they say what to do, and no - # workspace follows one. Every flag not here modifies a launch that still - # needs a spec, which is the distinction the scan below turns on -- `dl --ls` - # is finished and `dl --devcontainer robot` has not started. + # The flags a workspace spec may still follow: they modify a launch instead + # of being one. Every other flag ends the line, and that direction is the + # load-bearing half -- listing the flags that *end* it instead put ten flags + # in the wrong arm at once, because the ones nobody thinks to list are all on + # that side: the command group's hidden members (--repos, --update-cache, + # --completion-data), the five that modify a command already on the line + # (--json, --size, --yes, --force, --force-worktrees), and the two retired + # spellings. `dl --json my-workspace` is a clap error and `dl --repos + # my-workspace` answers "--repos takes no workspace", so completing a name + # after either is completing onto a refusal. # - # Derived rather than judged: these are exactly the flags dl declares in - # clap's `what` group, whose members are mutually exclusive because each one - # *is* the command, plus the help and version clap generates. - # `dl/tests/completion_tables.rs` diffs the two. - local command_opts="--ls --install --refresh --prune --reconcile --purge --herdr-shell --claude-profiles --help -h --version" + # Derived rather than judged, from three tables that are each already pinned: + # the grammar's flags, minus clap's `what` group, minus the hidden ones, + # minus `NOT_OFFERED_FIRST`. `dl/tests/completion_tables.rs` does that + # subtraction and diffs the answer against this line. + local spec_follows="--rm --devcontainer --claude-profile" if [[ "$cmd" == aid ]]; then - # The three aid answers itself, before it rewrites anything into a dl - # line. Every other aid flag is read ahead of the spec and passed - # through, so the spec is still to come. - command_opts="--help -h --version" + # aid's own, from `parse_aid_args`: it reads an agent flag, a remote + # control flag or a dl value option and keeps looking for the spec. The + # three it answers itself (--help, -h, --version) are absent because they + # end the line, and so is an unknown flag -- aid cannot tell whether one + # takes a value, so on `aid --unknown-taking-a-value foo owner/repo` it + # calls `foo` the spec, and completing a slot aid itself cannot place is + # worse than completing nothing. + spec_follows="--claude --codex --gemini --remote-control --remote --no-remote-control --no-remote --devcontainer --claude-profile" fi # After --claude-profile, offer the profile directories that exist. Read off the @@ -194,7 +203,7 @@ _dl_completion() { # The words strictly before `cur` are indices 1 through word_count-2, which # holds whether or not the line ends in a space: the trailing-space branch # above incremented word_count without appending to `words`. - local position=0 named_a_command=0 scan=1 scanned + local position=0 ends_here=0 modified=0 scan=1 scanned while (( scan <= word_count - 2 )); do scanned="${words[scan]}" if [[ "$scanned" == "--" ]]; then @@ -202,34 +211,43 @@ _dl_completion() { # is the user's shell to complete and not ours. return 0 fi - if [[ " ${value_opts} " == *" ${scanned} "* ]]; then - # Its value is not a positional word, so step over the pair. - (( scan += 2 )) - continue - fi if [[ "$scanned" == -* ]]; then - if [[ " ${command_opts} " == *" ${scanned} "* ]]; then - named_a_command=1 + if [[ " ${spec_follows} " == *" ${scanned} "* ]]; then + modified=1 + else + ends_here=1 + fi + if [[ " ${value_opts} " == *" ${scanned} "* ]]; then + # Its value is not a positional word, so step over the pair. + (( scan += 2 )) + else + (( scan++ )) fi - (( scan++ )) continue fi (( position++ )) (( scan++ )) done - # A line that already named a command takes neither a workspace nor a verb. - # This is what used to be a guard on the first word starting with `--`, which - # could not tell `dl --ls` from `dl --rm`. - if (( named_a_command )); then + # A line carrying a flag no spec follows takes neither a workspace nor a + # verb. This is what used to be a guard on the first word starting with + # `--`, which could not tell `dl --ls` from `dl --rm`. + if (( ends_here )); then return 0 fi # The spec's position: flags, workspaces, repos, owners, or paths. if (( position == 0 )); then - # Global flags + # Flags. Once one modifier is on the line the line is a launch, so the + # only flags that can still precede the spec are the other modifiers: + # `dl --rm --ls` is refused, and `aid --codex --help` is not aid's help + # (that is `argv[0]`) but an unknown option handed to dl. if [[ ${cur} == -* ]]; then - COMPREPLY=( $(compgen -W "${global_opts}" -- ${cur}) ) + if (( modified )); then + COMPREPLY=( $(compgen -W "${spec_follows}" -- ${cur}) ) + else + COMPREPLY=( $(compgen -W "${global_opts}" -- ${cur}) ) + fi return 0 fi diff --git a/rust/dl/tests/completion_tables.rs b/rust/dl/tests/completion_tables.rs index 022a279f..6fc9f7f6 100644 --- a/rust/dl/tests/completion_tables.rs +++ b/rust/dl/tests/completion_tables.rs @@ -267,35 +267,73 @@ fn aid_agents(rewrite: &str) -> BTreeSet { } /// A `const NAME: &[&str] = &["a", "b"];` list of flag spellings. +/// +/// An entry may be another `const` rather than a literal — `REMOTE_CONTROL_FLAGS` +/// names `REMOTE_CONTROL_FLAG` so the flag and the list of its spellings cannot +/// disagree — so a bare identifier is resolved to the string that `const` is +/// declared as. Every entry has to resolve to exactly one flag, asserted against +/// the number of entries actually written: reading one of two spellings and +/// comparing the short list against the script is the silent pass this whole file +/// exists to prevent, and it is what happened before the count went in. fn aid_flag_list(rewrite: &str, name: &str) -> BTreeSet { - let flags: BTreeSet = slice_body(rewrite, name) + let body = slice_body(rewrite, name); + let entries: Vec<&str> = body .split(',') - .filter_map(|part| { - part.trim() - .strip_prefix('"') - .and_then(|rest| rest.strip_suffix('"')) - .map(str::to_owned) - }) + .map(str::trim) + .filter(|part| !part.is_empty()) + .collect(); + let flags: BTreeSet = entries + .iter() + .map( + |part| match part.strip_prefix('"').and_then(|r| r.strip_suffix('"')) { + Some(literal) => literal.to_owned(), + // A named const: `const THAT_NAME: &str = "--flag";`. + None => named_str_const(rewrite, part), + }, + ) .collect(); - assert!(!flags.is_empty(), "{name} lists at least one flag"); + assert_eq!( + flags.len(), + entries.len(), + "one flag was read per entry of {name}; {entries:?} became {flags:?}" + ); flags } +/// The string a `const NAME: &str = "...";` in aid's rewrite is declared as. +fn named_str_const(rewrite: &str, name: &str) -> String { + let after = rewrite + .split_once(&format!("const {name}: &str = \"")) + .unwrap_or_else(|| panic!("aid's rewrite declares {name} as a &str const")) + .1; + after + .split_once('"') + .unwrap_or_else(|| panic!("{name}'s literal ends")) + .0 + .to_owned() +} + /// Flags the grammar accepts that the completion deliberately does not offer for a /// first argument, each with the reason it is left out. /// /// **Hand-written, and that is the point.** Everything else is derived, so a flag /// added to the grammar breaks the test below until somebody either completes it or /// writes it here — which is the decision this guard exists to force. The common -/// thread is position: the completion offers this table where a *command* goes, and -/// none of these is one. They modify a line that already named one, and the script -/// stops completing a line that has named one, so there is nowhere to offer them. +/// thread is position: each needs something already on the line, so none is ever +/// the first word. /// -/// A leading `--` no longer ends completion by itself — the script tells a flag -/// that is the command from one that modifies a launch, which is what -/// [`the_flags_that_end_a_line_are_the_grammars_own_command_group`] pins — so a -/// modifier reaches this table now (`dl --rm --`). These five still do not, -/// because each modifies a *command* flag and the script offers nothing after one. +/// A leading `--` no longer ends completion by itself — the script tells a flag a +/// spec may follow from one that ends the line, which is what +/// [`the_flags_a_spec_may_follow_are_the_launch_modifiers_the_grammar_leaves_over`] +/// pins — so a launch modifier reaches this table now (`dl --rm --` offers +/// the other modifiers). These five still do not, and the reason is not one +/// reason: `--json` and `--size` carry `requires = "ls"` and are a clap error +/// without it; `--yes` and `--force-worktrees` are refused by name as meaningless +/// for a workspace command; and a *leading* `--force` is not a modifier at all but +/// the workspace slot itself — `force_placement` reads `--force` with no word +/// before it as `ForcePlace::WorkspaceSlot`, so `dl --force my-ws` answers +/// "Unknown workspace '--force'". All four refusals are what +/// `test_only_a_flag_a_spec_can_follow_is_completed_past` quotes. const NOT_OFFERED_FIRST: [(&str, &str); 5] = [ ( "--json", @@ -426,58 +464,93 @@ fn the_flags_a_value_follows_are_the_grammars_value_taking_flags() { } #[test] -fn the_flags_that_end_a_line_are_the_grammars_own_command_group() { - // The script has to tell a flag that *is* the command from one that modifies a - // launch, because that is what says whether a workspace spec is still to come: - // `dl --ls` is finished and `dl --devcontainer robot` has not started. Get it - // wrong in one direction and `dl --rm ` offers nothing; wrong in the other - // and `dl --ls ` offers every workspace to a command that takes none. +fn the_flags_a_spec_may_follow_are_the_launch_modifiers_the_grammar_leaves_over() { + // The script has to tell a flag a workspace spec still follows from one that + // ends the line, because that is what decides whether to complete a spec at + // all: `dl --devcontainer robot` has not started and `dl --ls` is finished. + // + // Listed in the direction of the exceptions, and derived by subtraction, for + // the reason the script's own comment gives: the flags nobody thinks to list + // are all on the ending side, so a hand-written list of *those* is the one + // that silently drifts. Three subtrahends, each already pinned by a test in + // this file or forced by `NOT_OFFERED_FIRST`: + // + // every flag - clap's `what` group - the hidden ones - NOT_OFFERED_FIRST // - // clap's `what` group is the original, and it is one rather than a coincidence: - // its members are mutually exclusive precisely because each one is the whole - // command. Hidden flags are dropped for the same reason as everywhere else here - // -- a spelling `--help` does not show is not one the script offers -- and the - // two retired ones are deliberately outside the group anyway. + // `what` because each of its members is the whole command; hidden because a + // spelling `--help` does not show is one only a retired build still answers + // (`--stop`, `--autorm`) or an internal re-entry (`--repos`, + // `--completion-data`, `--update-cache`), and none of those takes a + // workspace; `NOT_OFFERED_FIRST` because each of those five modifies a + // *command* that is already on the line, which is the very thing whose + // presence ends it. + // + // What survives is the launch modifiers, and the subtraction is what makes + // that a derivation rather than a second opinion. let script = completion_script(); let grammar = argument_grammar(); - let mut expected: BTreeSet = grammar_flags(&grammar) + let withheld: BTreeSet<&str> = NOT_OFFERED_FIRST.iter().map(|(flag, _)| *flag).collect(); + let expected: BTreeSet = grammar_flags(&grammar) .iter() - .filter(|flag| flag.names_a_command && !flag.hidden) + .filter(|flag| { + !flag.names_a_command && !flag.hidden && !withheld.contains(flag.long.as_str()) + }) .map(|flag| flag.long.clone()) .collect(); - // clap generates these, and both end a line: `dl --help owner/repo` prints the - // help and opens nothing. - expected.insert("--help".to_owned()); - expected.insert("-h".to_owned()); + assert!( + !expected.is_empty(), + "the subtraction leaves something: a spec follows at least one flag" + ); assert_eq!( - assigned(&script, "local command_opts="), + assigned(&script, "local spec_follows="), expected, - "the completion script's command flags have drifted from the grammar's \ - `what` group" + "the completion script's launch modifiers have drifted from the grammar" ); assert!( - assigned(&script, "local command_opts=").is_subset(&dl_first_argument_flags(&script)), - "a flag that ends a line is a flag that can be tabbed to in the first place" + expected.is_subset(&dl_first_argument_flags(&script)), + "a flag a spec follows is a flag that can be tabbed to in the first place" ); } #[test] -fn a_flag_beside_a_workspace_is_never_one_that_ends_the_line() { - // The two tables read the same word differently, so a word in both would be a - // contradiction: `ws_cmds` offers `--rm` *after* a spec, which only makes sense - // for a flag the scan lets a spec follow. +fn every_flag_whose_value_is_completed_is_one_a_spec_may_follow() { + // The scan steps over a value option *and its value* and then keeps looking + // for the spec, so a value option missing from `spec_follows` would eat its + // value and end the line in the same breath -- `dl --devcontainer robot + // owner/repo` would complete nothing, which is half of the defect this whole + // change fixes. Checked for both binaries, because aid's table is its own. + let script = completion_script(); + + let values = assigned(&script, "local value_opts="); + for table in ["local spec_follows=", "spec_follows="] { + let follows = assigned(&script, table); + for flag in &values { + assert!( + follows.contains(flag), + "{flag} takes a value that {table} does not let a spec follow" + ); + } + } +} + +#[test] +fn a_flag_offered_beside_a_workspace_is_one_a_spec_could_have_followed() { + // `ws_cmds` offers `--rm` *after* a spec and `spec_follows` lets one come + // after it, and both readings have to hold of the same word: `dl --rm ` + // and `dl --rm` are the same request written either way round, so a + // flag in one table and not the other would answer them differently. let script = completion_script(); - let ends_the_line = assigned(&script, "local command_opts="); + let follows = assigned(&script, "local spec_follows="); let (_, beside) = dl_workspace_words(&script); + assert!(!beside.is_empty(), "the list offers at least one flag"); for flag in &beside { assert!( - !ends_the_line.contains(flag), - "{flag} is offered beside a workspace and also listed as ending the \ - line, and it cannot be both" + follows.contains(flag), + "{flag} is offered beside a workspace but no spec may follow it" ); } } @@ -524,23 +597,41 @@ fn aid_offers_one_flag_per_agent_it_can_start() { } #[test] -fn the_only_aid_flags_that_end_a_line_are_the_three_aid_answers_itself() { - // aid's half of `the_flags_that_end_a_line_are_the_grammars_own_command_group`. - // There is no group to derive it from -- aid's grammar is not a clap grammar -- - // but there is already a hand-written table of the flags aid answers before it - // rewrites anything, and those are exactly the flags no workspace follows. Every - // other aid flag, agent flags and unknown pass-throughs alike, is read ahead of - // the spec, so the spec is still to come. +fn the_aid_flags_a_spec_may_follow_are_the_ones_parse_aid_args_reads_past() { + // aid's half of `the_flags_a_spec_may_follow_are_the_launch_modifiers_...`, and + // derived the same way even though aid's grammar is not a clap one: these are + // exactly the three tables `parse_aid_args` reads and then keeps looking for the + // spec -- an agent flag, either polarity of the remote-control flag, or a dl + // value option. + // + // The three flags aid answers itself are absent, and so is an unknown flag: aid + // passes one through to dl as a boolean option, so on `aid --unknown-taking-a- + // value foo owner/repo` it calls `foo` the spec, and no completion is honest + // about a slot aid itself cannot place. let script = completion_script(); + let rewrite = aid_rewrite(); - let expected: BTreeSet = AID_FLAGS_BESIDE_THE_AGENTS + let mut expected: BTreeSet = aid_agents(&rewrite) .iter() - .map(|(flag, _)| (*flag).to_owned()) + .map(|agent| format!("--{agent}")) .collect(); + for table in [ + "REMOTE_CONTROL_FLAGS", + "NO_REMOTE_CONTROL_FLAGS", + "DL_VALUE_OPTIONS", + ] { + expected.extend(aid_flag_list(&rewrite, table)); + } assert_eq!( - assigned(&script, "command_opts="), + assigned(&script, "spec_follows="), expected, - "aid's command flags have drifted from the flags aid answers itself" + "aid's launch modifiers have drifted from what `parse_aid_args` reads past" ); + for (flag, _) in AID_FLAGS_BESIDE_THE_AGENTS { + assert!( + !expected.contains(flag), + "{flag} is one aid answers itself, so no spec follows it" + ); + } } diff --git a/test/test_bash_completion.py b/test/test_bash_completion.py index bdba7bff..5317f487 100644 --- a/test/test_bash_completion.py +++ b/test/test_bash_completion.py @@ -323,13 +323,20 @@ def test_an_agent_flag_does_not_move_the_workspace_spec(self): offered a spec at word two alone, which made every `aid --codex`, `aid --claude` and `aid --gemini` line complete nothing at all. """ - bare = self.run_completion("aid my-") - assert bare, "the line the flag is added to completes something to begin with" - for flag in ["--claude", "--codex", "--gemini"]: - assert self.run_completion(f"aid {flag} my-") == bare, ( - f"`aid {flag} my-` completes differently from `aid my-`, but the " - "flag is read before the spec and does not stand in its place" - ) + # Both halves of what the function answers, because "the same" includes + # whether the cursor is left against the `/` -- an owner completed with a + # trailing space is a different gesture from one without, and COMPREPLY + # cannot show the difference. `run_completion` alone would pass a script + # that appended a space here and not there. + for spec in ["my-", "", "my-org/", "my-org/my-repo@", "./"]: + bare = self.run_completion_with_options(f"aid {spec}") + assert bare[0], f"`aid {spec}` completes something to begin with" + for flag in ["--claude", "--codex", "--gemini"]: + assert self.run_completion_with_options(f"aid {flag} {spec}") == bare, ( + f"`aid {flag} {spec}` completes differently from `aid {spec}`, " + "but the flag is read before the spec and does not stand in its " + "place" + ) def test_a_value_option_and_its_value_do_not_move_the_workspace_spec(self): """`dl --devcontainer robot ` completes its spec too. @@ -348,10 +355,15 @@ def test_a_leading_modifier_leaves_the_verb_where_it_was(self): `--rm` modifies a launch rather than being one, so the spec is still to come after it and a verb after that. + + The anchor is not decoration: without it both sides are `[]` when the + verb branch never fires, and the test passes against a script that + completes nothing at all. Mutating `position == 1` to `position == 99` + was green before it was added. """ - assert self.run_completion("dl --rm my-workspace ") == self.run_completion( - "dl my-workspace " - ) + verbs = self.run_completion("dl my-workspace ") + assert "stop" in verbs, "the line the flag is added to offers verbs at all" + assert self.run_completion("dl --rm my-workspace ") == verbs def test_a_command_flag_ends_the_line_whatever_follows_it(self): """A flag that *is* the command takes neither a workspace nor a verb. @@ -366,9 +378,76 @@ def test_a_command_flag_ends_the_line_whatever_follows_it(self): "command and no workspace follows one" ) + def test_only_a_flag_a_spec_can_follow_is_completed_past(self): + """A workspace is offered after a flag only if the CLI takes one there. + + The ten flags below all refuse a workspace, and each refusal is the + proof: `dl --repos my-ws` answers "--repos takes no workspace", + `dl --yes my-ws` answers "--yes means nothing for a workspace command", + and `dl --json my-ws` is a clap error about the missing `--ls`. Tabbing + to a name and then being refused for something you never typed is worse + than no completion at all, which is the same bar + `test_the_completion_offers_only_names_a_launch_accepts` holds the + profile names to. + + Three groups, and they are why the scan's default arm has to be "this + flag ends the line" rather than "a spec is still to come": the hidden + members of the command group, the five flags that modify a command + already on the line, and the two retired spellings. + """ + for flag in [ + "--repos", + "--completion-data", + "--update-cache", + "--json", + "--size", + "--yes", + "--force", + "--force-worktrees", + "--stop", + "--autorm", + ]: + assert self.run_completion(f"dl {flag} ") == [], ( + f"`dl {flag} ` offers a workspace, and `dl {flag} ` is a " + "line the CLI refuses" + ) + + def test_a_spec_still_follows_the_flags_that_modify_a_launch(self): + """The other side of that rule: the three dl flags a spec does follow. + + Named as the whole set rather than sampled, because the fix turns on + the default arm being a refusal -- so this is the list of exceptions, + and a flag missing from it completes nothing. + """ + for flag in ["--rm", "--devcontainer robot", "--claude-profile work"]: + assert self.run_completion(f"dl {flag} my-") == ["my-org/"], ( + f"`dl {flag} my-` offers no spec, but the flag modifies a " + "launch and a workspace is what comes next" + ) + + def test_an_unknown_leading_flag_completes_nothing_for_either_binary(self): + """A flag neither grammar knows ends the line rather than shifting it. + + For dl because clap refuses the flag outright, and for aid because it + cannot know whether an unknown flag takes a value: `parse_aid_args` + reads one as a boolean dl option, so on + `aid --unknown-taking-a-value foo owner/repo` it calls `foo` the spec. + Completing nothing is the honest answer to a line whose slots aid + itself cannot place. + """ + for line in ["dl --bogus my-", "aid --bogus my-", "dl --bogus ", "aid --bogus "]: + assert self.run_completion(line) == [] + def test_nothing_is_completed_past_the_double_dash(self): """After `--` the words are the command run inside the workspace.""" - for line in ["dl my-workspace -- ", "dl my-workspace -- ls my-", "dl --rm my-ws -- "]: + # Anchored the same way: each line without its `--` completes something, + # so an empty answer is the `--` and not a script that answers nothing. + for line, before in [ + ("dl my-workspace -- ", "dl my-workspace "), + ("dl my-workspace -- ls my-", "dl my-workspace "), + ("dl --rm my-ws -- ", "dl --rm "), + ]: + assert self.run_completion(before), f"`{before}` completes something" assert self.run_completion(line) == [] def test_an_aid_prompt_is_never_a_verb_however_the_line_began(self): From 207a7dbe098428adb2a405cba6ffbc2089500856 Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Wed, 9 Sep 2026 13:24:38 +0000 Subject: [PATCH 3/4] docs: the ending arm means no workspace, and --install takes a path Types axis: the field was documented as "the flag *is* the command, so no workspace follows it", which is true of --install and reads as though no *word* follows. `dl --install []` takes an optional path, and nothing completes it. Unchanged by this branch and left alone: the path branch lives in the spec position, so completing it is a second exception rather than a wider table. Claude-Session: https://claude.ai/code/session_01GxeUVm3R579Tqai6GwoEmb --- rust/devlaunch-core/completions/dl.bash | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/rust/devlaunch-core/completions/dl.bash b/rust/devlaunch-core/completions/dl.bash index 2797ad0a..23a7e61d 100644 --- a/rust/devlaunch-core/completions/dl.bash +++ b/rust/devlaunch-core/completions/dl.bash @@ -99,6 +99,13 @@ _dl_completion() { # the grammar's flags, minus clap's `what` group, minus the hidden ones, # minus `NOT_OFFERED_FIRST`. `dl/tests/completion_tables.rs` does that # subtraction and diffs the answer against this line. + # + # "Ends the line" means no *workspace* follows, which is not quite the same + # as no word: `dl --install []` takes an optional path. Nothing + # completes that path, here or before this scan existed, and offering it + # would mean a second exception rather than a wider `spec_follows` -- the + # thing that follows is not a spec, and the branch below that handles `./` + # is inside the spec position. local spec_follows="--rm --devcontainer --claude-profile" if [[ "$cmd" == aid ]]; then # aid's own, from `parse_aid_args`: it reads an agent flag, a remote From 31d49fc552726261b10371880437c858555b6452 Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Wed, 9 Sep 2026 13:25:29 +0000 Subject: [PATCH 4/4] docs: record the completion position rule CHANGELOG under [Unreleased], and a docs/workspace-tools.md section under Shell completion. The rationale worth keeping is why the table lists the flags a spec may follow rather than the flags that end the line: the endings are the side nobody thinks to list, and each of them refuses a workspace for a different reason. Claude-Session: https://claude.ai/code/session_01GxeUVm3R579Tqai6GwoEmb --- CHANGELOG.md | 33 +++++++++++++++++++++++++++++++ docs/workspace-tools.md | 44 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d6682dd..3b085f13 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,39 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **A flag before the workspace spec no longer stops tab completion.** + `aid --codex owner/repo` offered nothing at all, and neither did + `aid --claude`, `aid --gemini` or `dl --devcontainer robot owner/repo`. + The completion script found the spec by counting words from the command, word + two for the spec and word three for a verb, and neither grammar works that + way: `parse_aid_args` reads aid's leading flags and calls the first word that + is not one the spec, and dl is clap, which puts options anywhere among the + positional words. + + The script now scans the words before the cursor and counts the positional + ones, stepping over a value option with its value and stopping at `--`, so + the spec is wherever it actually lands. `dl --rm my-ws` completes a + workspace, `dl --rm my-ws ` completes a verb, and `aid --codex my-ws` + completes exactly what `aid my-ws` does, trailing space included. + + That needs a distinction the old guard could not make, since it ended + completion on any leading `--`: a flag a spec may follow against one that + ends the line. The table lists the first, three flags for dl and nine for + aid, and every other flag ends the line. That direction is deliberate and it + is where the first attempt went wrong: listing the endings instead put ten + flags in the wrong arm, because the ones nobody thinks to list are all on + that side. `dl --repos my-workspace` answers "--repos takes no workspace", + `dl --json my-workspace` is a clap error about the missing `--ls`, and + `dl --force my-workspace` answers "Unknown workspace '--force'" because a + leading `--force` is the workspace slot itself. Completing a name onto any of + those is worse than completing nothing, so the default arm is the refusal. + + Both tables are derived rather than hand-judged, and + `rust/dl/tests/completion_tables.rs` diffs them: dl's is every flag the + grammar declares minus clap's `what` group, minus the hidden ones, minus the + five that need something already on the line, and aid's is the three tables + `parse_aid_args` reads past. + - **Editing prose under `.devcontainer/` no longer throws away the prebuilt container.** Opening this repository with `dl` pulls a published image instead of building one, and it had gone back to building. The prebuild tag is a hash diff --git a/docs/workspace-tools.md b/docs/workspace-tools.md index 71245b35..3f0b4122 100644 --- a/docs/workspace-tools.md +++ b/docs/workspace-tools.md @@ -1502,6 +1502,50 @@ After running `dl --install`, tab completion offers: - File/directory paths when starting with `./`, `/`, or `~` - All global flags (`--ls`, `--install`, etc.) and workspace commands +### A flag before the spec does not move it + +Both command lines take flags ahead of the workspace, and the completion reads +them rather than counting words: + +``` +$ aid --codex kin +kinisi-robotics/ +$ dl --devcontainer robot kin +kinisi-robotics/ +``` + +Counting is what this used to do, and it put the spec at the second word alone, +so every one of those lines completed nothing. The rule each command actually +follows is different. aid reads its leading flags and calls the first word that +is not one the spec, which is `parse_aid_args`, and dl is a clap grammar, which +lets an option sit anywhere among the positional words. + +The flags a spec may follow are the ones that modify a launch: `--rm`, +`--devcontainer` and `--claude-profile` for `dl`, and for `aid` those two plus +the agent flags and either polarity of `--remote-control`. Every other flag ends +the line, and nothing is offered after one: + +``` +$ dl --ls +$ dl --json +$ dl --repos +``` + +Listing the flags a spec may follow, rather than the flags that end the line, is +the load-bearing choice. The flags nobody thinks to list are all on the ending +side, and each of them refuses a workspace for a different reason: `--repos` +answers "--repos takes no workspace", `--json` is an error about the missing +`--ls` it requires, `--yes` is refused as meaningless for a workspace command, +and a leading `--force` is not a modifier at all but the workspace slot itself, +so `dl --force my-ws` answers "Unknown workspace '--force'". Tabbing to a name +and then being refused for a word you never typed is worse than no completion, +which is the same bar the profile names are held to under +[Naming a profile](#naming-a-profile). + +Once one modifier is on the line the only flags still offered are the other +modifiers, since `dl --rm --ls` is refused and `aid --codex --help` is not aid's +help but an unknown option handed to dl. + ### Owners come before workspace ids, and why The first word of a `dl` line can be two different things, a spec or the id of a