Skip to content

Fix mcp list-tools tab completion offering completions for second positional arg#30221

Merged
pelikhan merged 5 commits intomainfrom
copilot/fix-integration-cli-completion
May 5, 2026
Merged

Fix mcp list-tools tab completion offering completions for second positional arg#30221
pelikhan merged 5 commits intomainfrom
copilot/fix-integration-cli-completion

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 4, 2026

mcp list-tools only accepts one positional argument (cobra.MaximumNArgs(1)), but its ValidArgsFunction was CompleteWorkflowNames directly — causing workflow names to be offered as completions for the second positional argument as well.

Change

Wrap CompleteWorkflowNames to short-circuit when len(args) > 0:

ValidArgsFunction: func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
    if len(args) > 0 {
        return nil, cobra.ShellCompDirectiveNoFileComp
    }
    return CompleteWorkflowNames(cmd, args, toComplete)
},

This aligns completion behavior with the command's declared argument cardinality.

Tests

Added TestMCPListToolsValidArgsFunction in pkg/cli/mcp_list_tools_test.go with two sub-tests:

  • no_completions_when_first_arg_already_provided — asserts the guard returns no completions and ShellCompDirectiveNoFileComp when a positional arg is already present
  • completions_offered_for_first_arg — asserts at least one workflow name completion is returned when no args are present and workflows exist on disk

Copilot AI and others added 2 commits May 4, 2026 21:03
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/d3b0a7e5-dece-4a46-a108-a7eba486df24

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix failing GitHub Actions job for CLI completion Fix mcp list-tools tab completion offering completions for second positional arg May 4, 2026
Copilot AI requested a review from pelikhan May 4, 2026 21:13
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

Hey @copilot-swe-agent 👋 — great catch on the tab-completion bug for mcp list-tools! The fix is clean and the PR description clearly explains both the root cause and the solution.

One thing that would help get this across the finish line:

  • Add a testpkg/cli/mcp_list_tools.go is changed but there's no corresponding test update. A unit test covering the ValidArgsFunction wrapper (verifying it returns no completions when len(args) > 0) would give reviewers confidence the fix behaves as intended and guards against regressions.

If you'd like a hand, you can assign this prompt to your coding agent:

Add a unit test for the ValidArgsFunction wrapper introduced in pkg/cli/mcp_list_tools.go.
The test should cover:
1. When len(args) == 0 — completions should be returned (workflow names offered).
2. When len(args) > 0 — nil completions and cobra.ShellCompDirectiveNoFileComp should be returned.
Follow the existing test patterns in pkg/cli/ for cobra command completion tests.

Generated by Contribution Check · ● 1.6M ·

@pelikhan pelikhan marked this pull request as ready for review May 5, 2026 00:37
Copilot AI review requested due to automatic review settings May 5, 2026 00:37
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes shell tab-completion for gh aw mcp list-tools so workflow-name completions are only suggested for the first (and only) positional argument, matching cobra.MaximumNArgs(1).

Changes:

  • Wrapes CompleteWorkflowNames in a ValidArgsFunction guard that short-circuits when a positional arg is already present.
  • Adds GH_AW_INFO_VERSION: "1.0.40" to an env block in the metrics-collector.lock.yml workflow.
Show a summary per file
File Description
pkg/cli/mcp_list_tools.go Prevents workflow-name completions from being offered for a non-existent second positional argument.
.github/workflows/metrics-collector.lock.yml Adds missing GH_AW_INFO_VERSION env var in one step for consistency with other lock workflows/steps.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 2/2 changed files
  • Comments generated: 1

Comment thread pkg/cli/mcp_list_tools.go
Comment on lines +206 to +211
ValidArgsFunction: func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
if len(args) > 0 {
return nil, cobra.ShellCompDirectiveNoFileComp
}
return CompleteWorkflowNames(cmd, args, toComplete)
},
@pelikhan
Copy link
Copy Markdown
Collaborator

pelikhan commented May 5, 2026

@copilot review all comments

Copilot AI and others added 2 commits May 5, 2026 00:59
Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 5, 2026

@copilot review all comments

Added TestMCPListToolsValidArgsFunction in mcp_list_tools_test.go (commit 737af63) with two sub-tests:

  • no_completions_when_first_arg_already_provided — asserts that ValidArgsFunction returns no completions and ShellCompDirectiveNoFileComp when an arg is already present
  • completions_offered_for_first_arg — sets up a temp workflow directory and asserts at least one completion is returned for the first positional arg

@pelikhan pelikhan merged commit 73667dc into main May 5, 2026
@pelikhan pelikhan deleted the copilot/fix-integration-cli-completion branch May 5, 2026 01:26
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.

3 participants