Skip to content

fix: a flag before the workspace spec stopped tab completion - #592

Merged
blooop merged 4 commits into
mainfrom
autocomplete
Sep 9, 2026
Merged

fix: a flag before the workspace spec stopped tab completion#592
blooop merged 4 commits into
mainfrom
autocomplete

Conversation

@blooop

@blooop blooop commented Sep 9, 2026

Copy link
Copy Markdown
Owner

aid --codex owner/repo<TAB> offered nothing at all. Neither did aid --claude, aid --gemini, or dl --devcontainer robot owner/repo<TAB>.

What was wrong

The completion script found the workspace spec by counting words from the command: word two for the spec, word three for a verb. Neither grammar works that way.

  • aid reads its leading flags and calls the first word that is not one the spec (parse_aid_args), so --codex does not stand in the spec's place.
  • dl is clap, which puts options anywhere among the positional words.

The fix

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.

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 direction of that table is the load-bearing part, and it is where the first attempt went wrong. Listing the flags that end a line 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, the five that need something already on the line, and the two retired spellings. Each refuses a workspace, for four different reasons:

dl --repos my-workspace   -> "--repos takes no workspace"
dl --json my-workspace    -> error: required argument --ls was not provided
dl --yes my-workspace     -> "--yes means nothing for a workspace command"
dl --force my-workspace   -> "Unknown workspace '--force'"   (a leading --force
                             is the workspace slot itself, per force_placement)

All ten completed [] before, and the first attempt offered a workspace name for each. Tabbing to a name and then being refused for a word you never typed is worse than no completion, which is the bar test_the_completion_offers_only_names_a_launch_accepts already holds the profile names to. So the table lists the flags a spec may follow, three for dl and nine for aid, and the default arm is the refusal. That also covers an unknown flag for free.

Both tables are derived, not hand-judged

Per the standing rule on second copies of a fact, rust/dl/tests/completion_tables.rs diffs them:

  • dl: every flag the grammar declares, minus clap's what group, minus the hidden ones, minus NOT_OFFERED_FIRST. Leaves exactly --rm --devcontainer --claude-profile.
  • aid: the three tables parse_aid_args reads past, minus the three aid answers itself.

Writing that diff caught a hole in its own parser: 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. It now resolves the const and asserts one flag per entry.

Verification

Built test-first. Every behavioural claim above is a test, and the three interesting mutations are caught: flipping the default arm back, disabling the verb branch, disabling the spec branch. Two of the tests were themselves defective on the first pass and are fixed here, both found by mutation:

  • one compared two empty lists and passed with the verb branch mutated to position == 99;
  • the sameness the report asked for includes whether the cursor is left against the /, which COMPREPLY does not show, so the agent-flag test now diffs compopt too across five spec shapes.

cargo test --workspace, clippy -D warnings, cargo fmt, 757 python tests, pixi run lint all green.

Known gap, left alone

dl --install [<rc-file>] takes an optional path and nothing completes it, here or before. The path branch lives inside the spec position, so completing it is a second exception rather than a wider table. Noted in the script.

🤖 Generated with Claude Code

https://claude.ai/code/session_01GxeUVm3R579Tqai6GwoEmb

Summary by Sourcery

Fix workspace tab completion so it follows parsed positional arguments instead of assuming fixed word positions.

Bug Fixes:

  • Restore Bash workspace and command completion when launch flags or option values precede the workspace specification.
  • Prevent completion after flags, retired options, unknown options, or -- when the CLI will not accept a workspace in that position.

Enhancements:

  • Derive valid flag-position completion tables from the dl and aid argument grammars to keep completion behavior aligned with command parsing.

Documentation:

  • Document workspace completion with leading flags and clarify which flags may precede a workspace specification.

Tests:

  • Add behavioral coverage for agent flags, value options, leading modifiers, terminating flags, unknown options, --, and aid prompt handling.
  • Add grammar-consistency checks for value options and flags that may precede workspace specifications.

`aid --codex owner/repo<TAB>` offered no completions at all, and neither
did `dl --devcontainer robot owner/repo<TAB>`. 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 <TAB>` 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
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
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 [<rc-file>]` 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
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

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @blooop, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 4 days and 23 hours by commenting @sourcery-ai review. Upgrade to get a review now.

@sourcery-ai

sourcery-ai Bot commented Sep 9, 2026

Copy link
Copy Markdown

Reviewer's Guide

Fixes workspace and verb tab completion when aid or dl options precede the workspace spec by scanning positional arguments and using grammar-derived allowlists to distinguish launch modifiers from flags that terminate completion, with extensive regression and drift-detection coverage.

Flow diagram for option-aware workspace completion

flowchart TD
    A["Completion invoked"] --> B["Scan words before cursor"]
    B --> C{"Encounter --?"}
    C -- Yes --> D["Stop completion"]
    C -- No --> E{"Option takes a value?"}
    E -- Yes --> F["Skip option and its value"]
    E -- No --> G{"Flag in spec_follows?"}
    F --> G
    G -- No --> H["Refuse workspace and verb completion"]
    G -- Yes --> I["Mark launch modifier"]
    I --> J{"Count positional words"}
    G --> J
    J --> K{"Position 0?"}
    K -- Yes --> L["Complete flags or workspace spec"]
    K -- No --> M{"Position 1 and command is dl?"}
    M -- Yes --> N["Complete workspace verb"]
    M -- No --> O["No completion"]
Loading

File-Level Changes

Change Details Files
Reworked Bash completion to locate the workspace spec by parsing positional words instead of fixed word indexes.
  • Scan words before the cursor while skipping option values and stopping at --.
  • Track whether a flag permits a following spec versus terminating workspace completion.
  • Preserve verb completion after a workspace and prevent aid prompts from being treated as verbs.
  • Restrict flags offered after a modifier to valid launch modifiers.
rust/devlaunch-core/completions/dl.bash
Added drift-detection tests that derive valid completion flags from the CLI grammars.
  • Parse clap metadata, including command groups, hidden flags, value-taking options, and exclusions.
  • Resolve named string constants in aid flag tables and assert one resolved flag per entry.
  • Verify the script’s spec_follows tables match grammar-derived expectations for both binaries.
rust/dl/tests/completion_tables.rs
Added behavioral coverage for flag placement, completion boundaries, and completion equivalence.
  • Test aid agent flags and dl value options before workspace specs.
  • Test modifier-to-spec and modifier-to-verb completion, refusing command, unknown, retired, and incompatible flags.
  • Test -- termination and ensure aid prompts never expose workspace verbs.
  • Compare compopt behavior as well as candidates for aid completion parity.
test/test_bash_completion.py
Documented the corrected flag-before-spec completion behavior and its deliberate boundaries.
  • Added examples for aid and dl options preceding workspace specs.
  • Documented the allowlist semantics and why invalid or command flags end completion.
  • Noted the existing optional dl --install path-completion gap.
docs/workspace-tools.md
CHANGELOG.md

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@codecov

codecov Bot commented Sep 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.86%. Comparing base (908a24e) to head (31d49fc).

Additional details and impacted files
Flag Coverage Δ
python 42.98% <ø> (ø)
rust 95.13% <ø> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
shipped code (rust) 95.13% <ø> (-0.02%) ⬇️
harness and tooling (python) 42.98% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@blooop
blooop merged commit 0db5027 into main Sep 9, 2026
15 checks passed
@blooop
blooop deleted the autocomplete branch September 9, 2026 13:32
@blooop blooop mentioned this pull request Sep 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant