Skip to content

fix(ui): preserve MCPServer argument ordering when creating stdio servers#2080

Open
hawkli-1994 wants to merge 4 commits into
kagent-dev:mainfrom
hawkli-1994:fix-mcp-server-arg-order
Open

fix(ui): preserve MCPServer argument ordering when creating stdio servers#2080
hawkli-1994 wants to merge 4 commits into
kagent-dev:mainfrom
hawkli-1994:fix-mcp-server-arg-order

Conversation

@hawkli-1994

Copy link
Copy Markdown

Fixes #2078

Problem

When creating an stdio MCPServer from the UI, the generated CRD placed the package name after the additional arguments. For example, npx -y @modelcontextprotocol/server-map --stdio produced:

args:
  - -y
  - --stdio
  - '@modelcontextprotocol/server-map'

Fix

Build the args array in the same order shown in the command preview:
[commandPrefix..., packageName, additionalArgs...].

Changes

  • Extracted buildMCPServerArgs helper in ui/src/lib/toolUtils.ts.
  • Updated McpServerForm to use the helper for both npx and uvx flows.
  • Added unit tests covering the ordering, whitespace splitting, and empty argument filtering.

…vers

The UI was emitting the package name after the additional arguments,
so a command like
would produce args [-y, --stdio, package]. Build the args array in the
same order shown in the command preview:
[commandPrefix..., packageName, additionalArgs...].

Fixes kagent-dev#2078
Copilot AI review requested due to automatic review settings June 24, 2026 10:54
@hawkli-1994 hawkli-1994 requested a review from peterj as a code owner June 24, 2026 10:54
@github-actions github-actions Bot added the bug Something isn't working label Jun 24, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes stdio MCPServer CRD generation in the UI by ensuring the package name is placed before any additional arguments when constructing the args array, matching the command preview and expected executor semantics (e.g., npx -y <package> --stdio).

Changes:

  • Added a shared buildMCPServerArgs helper to consistently build stdio MCP server args in the correct order.
  • Updated McpServerForm to use the helper for both npx and uvx command flows.
  • Added unit tests validating ordering, whitespace splitting of the prefix, and filtering of empty args.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
ui/src/lib/toolUtils.ts Adds buildMCPServerArgs helper to construct stdio MCPServer args in preview-consistent order.
ui/src/lib/tests/toolUtils.test.ts Adds unit tests covering arg ordering and filtering behavior for the new helper.
ui/src/components/mcp/McpServerForm.tsx Switches stdio server creation to use the shared args builder for both npx and uvx.

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

Comment thread ui/src/lib/toolUtils.ts Outdated
Comment thread ui/src/lib/toolUtils.ts Outdated
Address Copilot review comments:
- Use consistent  spacing.
- Clarify that the executor (cmd) is supplied separately from the
  returned args array.
@peterj

peterj commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator

thanks for your PR! Could you sign the DCO and we can get this merged.

Comment thread ui/src/lib/toolUtils.ts Outdated

@peterj peterj left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

minor feedback

@mesutoezdil

Copy link
Copy Markdown
Contributor

quick quesstion: toolutils.ts:25 -- the old code always pushed packagename.trim() even when empty. the new guard silently skips an empty package name instead. is this intentional, or a side effect of the refactor?

peterj and others added 2 commits June 26, 2026 10:19
…kageName

Replace ArgPair object wrapper with plain strings for argPairs across the form, helper, and tests. buildMCPServerArgs now always includes packageName (validated by the form before calling).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] MCPServer argument ordering is incorrect

4 participants