Skip to content

fix(mcp): infer Codex URL-only MCP transport#2730

Open
xpfo-go wants to merge 1 commit into
farion1231:mainfrom
xpfo-go:fix/2492-codex-mcp-url-import
Open

fix(mcp): infer Codex URL-only MCP transport#2730
xpfo-go wants to merge 1 commit into
farion1231:mainfrom
xpfo-go:fix/2492-codex-mcp-url-import

Conversation

@xpfo-go
Copy link
Copy Markdown
Contributor

@xpfo-go xpfo-go commented May 12, 2026

Summary

  • Infer Codex MCP entries with a URL but no explicit type as SSE instead of stdio
  • Preserve the existing stdio default for entries without a URL
  • Add a regression test for importing a Codex URL-only MCP server such as the PyCharm /stream config

Fixes #2492

Test Plan

  • git diff --check
  • cargo fmt --manifest-path src-tauri/Cargo.toml --check
  • cargo test --manifest-path src-tauri/Cargo.toml --test import_export_sync

@DaveRoey96
Copy link
Copy Markdown

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@farion1231
Copy link
Copy Markdown
Owner

@codex review
@claude review

@claude
Copy link
Copy Markdown

claude Bot commented May 12, 2026

Claude finished @farion1231's task in 1m 1s —— View job


Code Review

LGTM. The change is small, focused, and correctly addresses the import failure for URL-only Codex MCP entries.

Behavior summary

  • infer_codex_mcp_type in src-tauri/src/mcp/codex.rs:271-285 preserves the explicit type when present, defaults to sse only when a non-empty url is set, and otherwise keeps the previous stdio default — so existing configs are unaffected.
  • The http/sse branch in the import loop (codex.rs:120-141) already knew how to extract url and http_headers, so the inferred sse path slots in cleanly with no further changes needed.
  • The new test at src-tauri/tests/import_export_sync.rs:610-642 reproduces the PyCharm /stream case from Codex importer should treat url MCP config as HTTP/SSE transport instead of defaulting to stdio. #2492 and asserts both the inferred type and round-tripped url. Validation at src-tauri/src/mcp/validation.rs:42-49 accepts sse with a non-empty URL, so the entry now passes instead of being rejected.

Notes (non-blocking)

  • Defaulting to sse rather than http is a judgment call. Either would have unblocked the import; sse matches the legacy Codex convention and the /stream URL shape, so this is defensible. Users can still adjust via the UI after import.

No blocking issues.


  • Read the PR diff against origin/main
  • Inspect src-tauri/src/mcp/codex.rs changes
  • Inspect src-tauri/tests/import_export_sync.rs changes
  • Check related code paths (validation, round-trip export)
  • Post final review
    · branch fix/2492-codex-mcp-url-import

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

ℹ️ 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".

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.

Codex importer should treat url MCP config as HTTP/SSE transport instead of defaulting to stdio.

3 participants