Skip to content

Preserve MCP launch argument boundaries on Windows - #465

Open
tninja wants to merge 2 commits into
mainfrom
kang_bug_fix
Open

Preserve MCP launch argument boundaries on Windows#465
tninja wants to merge 2 commits into
mainfrom
kang_bug_fix

Conversation

@tninja

@tninja tninja commented Jul 31, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes #464 by keeping CLI launch commands as argv lists from command resolution through MCP injection and terminal startup. This prevents Windows paths, arguments containing spaces, and inline MCP configuration from being corrupted by shell quote/split round trips.

  • Ghostel and Eat now pass program and arguments directly to their process APIs, while vterm quotes arguments only at its shell boundary.
  • Claude Code, Codex, GitHub Copilot CLI, and Open Interpreter append MCP configuration as discrete arguments; legacy string commands remain supported at the shared compatibility boundary.
  • Added regression coverage for Windows-style c:/... paths, spaced arguments, and backend-specific MCP configuration.

Verification

209 focused ERT tests pass, and strict byte compilation of the touched production files completes without warnings. The full suite produced only the same nine unrelated failures that also reproduce at the base commit.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7c8e7c3619

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread ai-code-mcp-agent.el
Comment on lines +68 to +69
(defun ai-code-mcp-agent-prepare-launch (backend working-dir argv)
"Return MCP launch metadata for BACKEND, WORKING-DIR, and ARGV."

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve legacy string inputs to the public launch helper

When existing integrations call the public ai-code-mcp-agent-prepare-launch with its previously documented command string, ai-code-mcp-agent--inject-argv passes that string to append, producing a list of character codes rather than an argv list; the shared command normalizer then rejects the launch as invalid. Since the repository's naming convention identifies this single-dash function as public, normalize legacy strings here before injection so existing custom backends continue to start.

AGENTS.md reference: AGENTS.md:L148-L150

Useful? React with 👍 / 👎.

@tninja

tninja commented Aug 4, 2026

Copy link
Copy Markdown
Owner Author

@copilot resolve the merge conflicts in this pull request

# Conflicts:
#	ai-code-backends-infra.el
#	ai-code-claude-code.el
#	ai-code-github-copilot-cli.el
#	ai-code-mcp-agent.el
#	test/test_ai-code-backends-infra.el
#	test/test_ai-code-codex-cli.el
#	test/test_ai-code-github-copilot-cli.el
#	test/test_ai-code-open-interpreter-cli.el

Co-authored-by: tninja <714625+tninja@users.noreply.github.com>

Copilot AI commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

Resolved all merge conflicts in commit 70b6cd1. The strategy was to keep the argv-list approach from this PR (which prevents Windows path/space corruption) while incorporating the bearer-token security and session registration that main added:

  • ai-code-mcp-agent.el: Kept --inject-argv (list-based), added token generation (ai-code-mcp--random-secret), session registration, and :env-vars return. Also added ai-code-mcp-agent-refresh-source-context from main. Updated --make-server-url to accept an optional session-id.
  • ai-code-backends-infra.el: Kept argv/launch-argv from this PR, added launch-env-vars extraction from main.
  • ai-code-claude-code.el / ai-code-github-copilot-cli.el: Kept :argv, added :env-vars passthrough.
  • Test files: Updated assertions to use member on argv lists instead of string-match-p on command strings, and added bearer_token_env_var + env-vars checks.

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.

MCP config path mangled on Windows: claude fails with "MCP config file not found"

2 participants