feat(mcp): #1167 prefix colliding MCP tool names

[IgnazioDS]

Summary

• add an opt-in mcp_config["include_server_in_tool_names"] flag to expose MCP tools as <server_name>_<tool_name>
• preserve the existing default behavior of raising on duplicate tool names across MCP servers
• keep invocation and meta resolution keyed to the original MCP tool name while logging the prefixed display name
• add regression coverage for collision handling, server-name sanitization, canonical meta resolution, and prefixed-name failure logging

Fixes #1167.

Validation

• uv run ruff check src/agents/agent.py src/agents/mcp/util.py tests/mcp/test_mcp_util.py
• uv run pytest tests/mcp/test_mcp_util.py -q
• uv run mypy src/agents/agent.py src/agents/mcp/util.py tests/mcp/test_mcp_util.py

Notes

• This refreshes the previously proposed server-prefix approach from closed PR feat(mcp): optional server-name prefix for colliding tool names #2442 against the current main branch and keeps the surface area limited to MCP config/util behavior plus focused tests.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9179a99593

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

[IgnazioDS]

Addressed the review thread in b9f25db.

Change made:

• preserve the readable normalized server prefix when it is unique
• add a stable hash-based suffix only when multiple server names normalize to the same prefix
• add a regression test covering GitHub MCP Server vs GitHub_MCP_Server

Validation:

• uv run ruff check src/agents/mcp/util.py tests/mcp/test_mcp_util.py
• PYTHONPATH=src uv run pytest tests/mcp/test_mcp_util.py -q
• uv run mypy src/agents/mcp/util.py --disable-error-code import-not-found --disable-error-code unused-ignore

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b9f25db053

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

[IgnazioDS]

Addressed the follow-up naming-collision thread in d710186.

Change made:

• switched prefixed MCP tool names to an unambiguous encoded form: mcp_<prefix_len>_<server_prefix><tool_name>
• kept the readable server-specific prefix visible in the final tool name
• retained the hash-based server-prefix disambiguation for server names that normalize to the same slug
• updated regression coverage for direct prefix collisions and tuple-level collisions

Validation:

• uv run ruff check src/agents/agent.py src/agents/mcp/util.py tests/mcp/test_mcp_util.py
• PYTHONPATH=src uv run pytest tests/mcp/test_mcp_util.py -q
• uv run mypy src/agents/agent.py src/agents/mcp/util.py --disable-error-code import-not-found --disable-error-code unused-ignore

[seratch]

Thanks for working on this. I think the feature is useful, but I don't think it's merge-ready yet because the new naming scheme can still generate invalid function tool names in two ways.

First, _server_tool_name_prefix() currently preserves non-ASCII alphanumeric characters. With include_server_in_tool_names=True, a server label like 天気サーバー becomes part of the public tool name and is then forwarded unchanged to the Responses payload. OpenAI function tool names need to stay within the allowed ASCII character set, so this path can fail at runtime even though the current tests are green.

Second, the prefixed name is never bounded to the API's 64-character limit. A sufficiently long server.name plus tool.name can easily produce a function name longer than 64 characters, and that overlong name is also passed through unchanged to the Responses tool definition.

So I think we need two additional safeguards before merging:

1. Normalize the server-derived prefix to an ASCII-safe subset only.
2. Enforce the 64-character limit on the final generated tool name, probably with a deterministic shortening/hash scheme so collisions remain impossible.

Once those two cases are covered, the overall approach looks good to me.

[IgnazioDS]

Thanks for the detailed review @seratch! Both issues addressed:

1. ASCII-only normalization — added char.isascii() guard in _server_tool_name_prefix so non-ASCII Unicode alphanumerics (CJK, Arabic, etc.) are now replaced with _ before being used as tool name prefixes:

char if (char.isascii() and char.isalnum()) or char in ("_", "-") else "_"

2. 64-character limit — _prefixed_tool_name now caps the result at 64 chars. When truncation is needed, a deterministic 8-char SHA-1 suffix is appended to keep names collision-resistant:

full_name = f"mcp_{len(tool_name_prefix)}_{tool_name_prefix}{tool_name}"
if len(full_name) <= 64:
    return full_name
hash_suffix = hashlib.sha1(full_name.encode("utf-8")).hexdigest()[:8]
truncated = full_name[: 64 - 9]
return f"{truncated}_{hash_suffix}"

[IgnazioDS]

Hi @seratch! I've addressed both points from your review in commit fa92be2:

1. ASCII normalization — _server_tool_name_prefix now uses char.isascii() and char.isalnum() instead of just char.isalnum(), so non-ASCII Unicode characters (CJK, Arabic, etc.) are replaced with _ and never reach the OpenAI function name.

2. 64-character limit — _prefixed_tool_name now checks len(full_name) <= 64 and, if over the limit, truncates to 55 chars and appends _{sha1[:8]} for collision resistance.

Would appreciate a re-review when you get a chance. Thanks!

[IgnazioDS]

Hi @seratch — thanks for the detailed review. Both concerns from your last review have been addressed:

1. Non-ASCII server prefix normalization (commit fa92be2)
_server_tool_name_prefix() now replaces every character that is not ASCII alphanumeric, _, or - with _, then strips leading/trailing separators:

normalized = "".join(
    char if (char.isascii() and char.isalnum()) or char in ("_", "-") else "_"
    for char in server_name
)

A server label like 天気サーバー becomes ____ → stripped to server (the fallback for all-separator results).

2. 64-character limit with deterministic hash shortening (commit fa92be2)
_safe_prefixed_tool_name() enforces the limit with a collision-safe SHA1 suffix:

if len(full_name) <= 64:
    return full_name
hash_suffix = hashlib.sha1(full_name.encode("utf-8")).hexdigest()[:8]
truncated = full_name[: 64 - 9]  # 9 = len("_") + 8 hash chars
return f"{truncated}_{hash_suffix}"

This ensures two different long names always produce distinct 64-char outputs.

Marking as ready for review — let me know if anything else needs adjusting.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fa92be2b65

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

[chatgpt-codex-connector]

Separate hashed prefixes from literal normalized prefixes

The disambiguation format for colliding server names (f"{normalized_prefix}_{hash_suffix}") is in the same namespace as normal single-server prefixes, so it can still collide in real inputs. For example, with server names foo, foo!, and foo_0beec7b5, the hashed prefix for foo becomes foo_0beec7b5_, which is identical to the literal normalized prefix for foo_0beec7b5; if both expose create_issue, get_all_function_tools still raises Duplicate tool names found across MCP servers even when include_server_in_tool_names=True.

Useful? React with 👍 / 👎.