lhumina_code/home
23
0
Fork 0
Code Issues 6 Pull requests Projects Releases Packages Wiki Activity Actions

[nu-demo] hero_agent tools payload blocks every LLM: 165 tools > 128 limit, dots in tool names violate regex, duplicate agent_run #153

New issue
Closed
opened 2026-04-24 01:50:02 +00:00 by mik-tf · 4 comments
mik-tf commented 2026-04-24 01:50:02 +00:00
Owner
Copy link

Symptom

When a message is triaged to the Tools path (e.g. asking hero_agent to "list files in this workspace") and hero_agent sends the full registered tool list to the backing LLM, every one of 7 configured LLM targets rejects the request:

Provider / model Error
Anthropic claude-haiku-4.5 (via aibroker) tools.60.custom.name: String should match pattern ^[a-zA-Z0-9_-]{1,128}$
OpenAI gpt-4o-mini (via aibroker) Invalid 'tools': array too long... got an array with length 165
Anthropic claude-3.5-sonnet No endpoints found
Google gemini-2.5-flash Duplicate function declaration found: agent_run
OpenAI gpt-4o-mini (OpenRouter direct) same 165 > 128
Groq llama-3.3-70b-versatile (skipped — key removed)
claude-3-5-sonnet-latest (OpenRouter) 128 tool limit

Result: every turn that requires a tool fails. The user sees an internal error, not an answer.

Root cause

Once hero_agent loads /home/driver/hero/var/agent/mcp.json pointing at hero_router's /mcp/:service endpoints (see sibling #153), each MCP service contributes its full OpenRPC method set:

  • Built-in hero_agent tools: ~58
  • MCP-registered tools from hero_osis, hero_books, hero_aibroker, hero_foundry, hero_indexer, hero_embedder, hero_proc: ~107 combined
  • Total: 165

Two class of violations in that payload:

A. Naming regex

The 58+ MCP tools have names like:

  • hero_osis.contact.list
  • rpc.discover
  • books.search

Anthropic requires ^[a-zA-Z0-9_-]{1,128}$ — dots are invalid. OpenAI has the same constraint. Hero's JSON-RPC method naming convention (namespace.verb) is fundamentally incompatible with tool-use naming rules.

B. Tool count

OpenAI has a hard limit of 128 tools per request (documented). Anthropic is softer but still pushes back past ~100. 165 is too many regardless.

C. Duplicates

The Gemini adapter emits agent_run twice — likely because two different registrations produce the same function name after Gemini's name-mangling step. Probably from the MCP round-trip creating duplicate entries.

Proposed fixes

Short term (demo-blocker)

  1. Sanitize tool names before emission:

    let sanitized = tool.name.replace('.', "_");  // "hero_osis.contact.list" → "hero_osis_contact_list"

    Apply in tool_router.rs when building the OpenAI/Anthropic payload. Preserve the original name internally for dispatch.

  2. Cap at 128 with a deterministic ordering: always_include first, then service-tagged ones that match the conversation, then the rest (trimmed). Honor the model's actual limit from config.aibroker_models metadata.

  3. Dedup by emitted name after step 1 (naming collisions should be reported, not silently dropped).

Medium term

  1. Per-call tool selection via a matcher (same idea as the existing groups system but actually used): only offer tools whose name prefix or keywords match the user's message. 10-20 relevant tools is plenty.

  2. Adapter-specific payloads — currently the llm_client pushes one payload shape to all providers. Each should get its own sanitization + limit.

Long term (proper)

  1. MCP tool namespacing — when pulling tools from an MCP server via hero_router, emit them under a prefix that's regex-safe by construction (books__search_query instead of hero_books.search.query). Update hero_router/src/server/mcp.rs so OpenRPC-to-MCP-tool conversion uses underscores.

Diagnostic log excerpt

claude-haiku-4.5 failed: LLM API error 400 Bad Request: ...
tools.60.custom.name: String should match pattern ^[a-zA-Z0-9_-]{1,128}$
tools.70.custom.name: String should match pattern ^[a-zA-Z0-9_-]{1,128}$
...
gpt-4o-mini failed: Invalid 'tools': array too long. Expected an array with maximum length 128, but got an array with length 165
gemini-2.5-flash failed: Duplicate function declaration found: agent_run

Related

  • #148 — nu-demo architecture index
  • #151 — triage classifier sends Hero questions to Knowledge path, masking this downstream issue in the most common user flow
  • #153 (or next) — mcp.json wiring + tool pull-through from hero_router

Verification

After fix:

  • Anthropic tool payload regex validation passes (all names match ^[a-zA-Z0-9_-]{1,128}$)
  • Count ≤ 128
  • No duplicate name entries
  • agent.chat({"message": "list files in /tmp", "model": "claude-haiku-4.5"}) returns a successful tool call + result

Signed-off-by: mik-tf

## Symptom When a message is triaged to the `Tools` path (e.g. asking hero_agent to "list files in this workspace") and `hero_agent` sends the full registered tool list to the backing LLM, **every one of 7 configured LLM targets rejects the request**: | Provider / model | Error | |---|---| | Anthropic `claude-haiku-4.5` (via aibroker) | `tools.60.custom.name: String should match pattern ^[a-zA-Z0-9_-]{1,128}$` | | OpenAI `gpt-4o-mini` (via aibroker) | `Invalid 'tools': array too long... got an array with length 165` | | Anthropic `claude-3.5-sonnet` | `No endpoints found` | | Google `gemini-2.5-flash` | `Duplicate function declaration found: agent_run` | | OpenAI `gpt-4o-mini` (OpenRouter direct) | same 165 > 128 | | Groq `llama-3.3-70b-versatile` | (skipped — key removed) | | claude-3-5-sonnet-latest (OpenRouter) | 128 tool limit | Result: every turn that requires a tool fails. The user sees an internal error, not an answer. ## Root cause Once `hero_agent` loads `/home/driver/hero/var/agent/mcp.json` pointing at hero_router's `/mcp/:service` endpoints (see sibling https://forge.ourworld.tf/lhumina_code/home/issues/153), each MCP service contributes its full OpenRPC method set: - Built-in hero_agent tools: ~58 - MCP-registered tools from hero_osis, hero_books, hero_aibroker, hero_foundry, hero_indexer, hero_embedder, hero_proc: ~107 combined - Total: **165** Two class of violations in that payload: ### A. Naming regex The 58+ MCP tools have names like: - `hero_osis.contact.list` - `rpc.discover` - `books.search` Anthropic requires `^[a-zA-Z0-9_-]{1,128}$` — **dots are invalid**. OpenAI has the same constraint. Hero's JSON-RPC method naming convention (`namespace.verb`) is fundamentally incompatible with tool-use naming rules. ### B. Tool count OpenAI has a hard limit of **128 tools per request** (documented). Anthropic is softer but still pushes back past ~100. 165 is too many regardless. ### C. Duplicates The Gemini adapter emits `agent_run` twice — likely because two different registrations produce the same function name after Gemini's name-mangling step. Probably from the MCP round-trip creating duplicate entries. ## Proposed fixes ### Short term (demo-blocker) 1. **Sanitize tool names** before emission: ```rust let sanitized = tool.name.replace('.', "_"); // "hero_osis.contact.list" → "hero_osis_contact_list" ``` Apply in `tool_router.rs` when building the OpenAI/Anthropic payload. Preserve the original name internally for dispatch. 2. **Cap at 128** with a deterministic ordering: `always_include` first, then service-tagged ones that match the conversation, then the rest (trimmed). Honor the model's actual limit from `config.aibroker_models` metadata. 3. **Dedup by emitted name** after step 1 (naming collisions should be reported, not silently dropped). ### Medium term 4. **Per-call tool selection** via a matcher (same idea as the existing `groups` system but actually used): only offer tools whose name prefix or keywords match the user's message. 10-20 relevant tools is plenty. 5. **Adapter-specific payloads** — currently the llm_client pushes one payload shape to all providers. Each should get its own sanitization + limit. ### Long term (proper) 6. **MCP tool namespacing** — when pulling tools from an MCP server via hero_router, emit them under a prefix that's regex-safe by construction (`books__search_query` instead of `hero_books.search.query`). Update `hero_router/src/server/mcp.rs` so OpenRPC-to-MCP-tool conversion uses underscores. ## Diagnostic log excerpt ``` claude-haiku-4.5 failed: LLM API error 400 Bad Request: ... tools.60.custom.name: String should match pattern ^[a-zA-Z0-9_-]{1,128}$ tools.70.custom.name: String should match pattern ^[a-zA-Z0-9_-]{1,128}$ ... gpt-4o-mini failed: Invalid 'tools': array too long. Expected an array with maximum length 128, but got an array with length 165 gemini-2.5-flash failed: Duplicate function declaration found: agent_run ``` ## Related - https://forge.ourworld.tf/lhumina_code/home/issues/148 — nu-demo architecture index - https://forge.ourworld.tf/lhumina_code/home/issues/151 — triage classifier sends Hero questions to Knowledge path, masking this downstream issue in the most common user flow - https://forge.ourworld.tf/lhumina_code/home/issues/153 (or next) — mcp.json wiring + tool pull-through from hero_router ## Verification After fix: - Anthropic tool payload regex validation passes (all names match `^[a-zA-Z0-9_-]{1,128}$`) - Count ≤ 128 - No duplicate `name` entries - `agent.chat({"message": "list files in /tmp", "model": "claude-haiku-4.5"})` returns a successful tool call + result Signed-off-by: mik-tf
mik-tf commented 2026-04-24 22:01:04 +00:00
Author
Owner
Copy link

Demo hotfix applied 2026-04-24

hero_agent::mcp_client.rs patched on development_mik_nu_demo:

  • Added sanitize_tool_name(raw) helper — replaces . with __ (so hero_osis_business.contact.list becomes hero_osis_business__contact__list), drops other illegal chars to _. Names now match ^[a-zA-Z0-9_-]+$.
  • McpTool gained original_name: Option<String> so calls back to MCP servers use the real name.
  • parse_tools_response populates both names; call_tool resolves either form.
  • Existing 128-tool truncation in agent.rs::route_tools already handles the count cap.

Verified: hero_agent_server rebuilt and running on herodemo. Tool name regex no longer rejects per-domain OSIS tools.

Prod-level fix needed

This sanitizer is the demo-time mitigation. The proper fix:

  1. Upstream the sanitizer to development (not just demo branch).
  2. Tool-list trimming policy: keep semantic+pattern routing as-is, but expose a mcp_tools.json whitelist so operators can pin core tools regardless of the relevance score.
  3. Prefer hyphens over underscores at the source — long-term, hero_osis OpenRPC names should use - separator (contact-list not contact.list) per MCP spec recommendations.
  4. Add a unit test: a tool-list with 200 candidates including dotted names must produce a payload that passes the LLM tool-name regex.

Tracking the upstream merge separately.

## Demo hotfix applied 2026-04-24 `hero_agent::mcp_client.rs` patched on `development_mik_nu_demo`: - Added `sanitize_tool_name(raw)` helper — replaces `.` with `__` (so `hero_osis_business.contact.list` becomes `hero_osis_business__contact__list`), drops other illegal chars to `_`. Names now match `^[a-zA-Z0-9_-]+$`. - `McpTool` gained `original_name: Option<String>` so calls back to MCP servers use the real name. - `parse_tools_response` populates both names; `call_tool` resolves either form. - Existing 128-tool truncation in `agent.rs::route_tools` already handles the count cap. **Verified:** `hero_agent_server` rebuilt and running on herodemo. Tool name regex no longer rejects per-domain OSIS tools. ## Prod-level fix needed This sanitizer is the demo-time mitigation. The proper fix: 1. **Upstream the sanitizer** to `development` (not just demo branch). 2. **Tool-list trimming policy:** keep semantic+pattern routing as-is, but expose a `mcp_tools.json` whitelist so operators can pin core tools regardless of the relevance score. 3. **Prefer hyphens over underscores at the source** — long-term, hero_osis OpenRPC names should use `-` separator (`contact-list` not `contact.list`) per MCP spec recommendations. 4. **Add a unit test:** a tool-list with 200 candidates including dotted names must produce a payload that passes the LLM tool-name regex. Tracking the upstream merge separately.
mik-tf commented 2026-04-25 15:13:37 +00:00
Author
Owner
Copy link

Merged to hero_agent/development 2026-04-25

Squash commit be302ed on hero_agent development:

fix(mcp_client): sanitize tool names + preserve original (#153)

PR #8 has been closed by the merge. The MCP tool-name sanitizer is now upstream — no need for the demo VM hotfix once a fresh deploy uses development everywhere.

Signed-off-by: mik-tf

## Merged to hero_agent/development 2026-04-25 Squash commit `be302ed` on hero_agent development: > fix(mcp_client): sanitize tool names + preserve original (#153) PR #8 has been closed by the merge. The MCP tool-name sanitizer is now upstream — no need for the demo VM hotfix once a fresh deploy uses development everywhere. Signed-off-by: mik-tf
mik-tf commented 2026-04-25 15:30:24 +00:00
Author
Owner
Copy link

Follow-up CI fix merged (PR #9)

The initial PR #8 merge (be302ed) introduced two regressions caught by CI:

  1. cargo fmt violation: inline if-else expression exceeded line length
  2. Missing field original_name in 13 McpTool { ... } initializers across tool_router.rs, semantic_router.rs (tests + routing helpers)
  3. clippy::while-let-on-iterator lint on sanitize_tool_name's loop

PR #9 (commits f9add73, 5cc3d24) addressed all three. hero_agent/development is at 5bea19b and CI green.

The lesson recorded in memory: when adding a required field to a struct used across the codebase, run cargo build --workspace (not just the target binary) before merging. CI catches this but it should never have shipped.

Signed-off-by: mik-tf

## Follow-up CI fix merged (PR #9) The initial PR #8 merge (`be302ed`) introduced two regressions caught by CI: 1. `cargo fmt` violation: inline `if`-else expression exceeded line length 2. Missing field `original_name` in 13 `McpTool { ... }` initializers across `tool_router.rs`, `semantic_router.rs` (tests + routing helpers) 3. `clippy::while-let-on-iterator` lint on `sanitize_tool_name`'s loop PR [#9](https://forge.ourworld.tf/lhumina_code/hero_agent/pulls/9) (commits `f9add73`, `5cc3d24`) addressed all three. `hero_agent/development` is at `5bea19b` and CI green. The lesson recorded in memory: when adding a required field to a struct used across the codebase, run `cargo build --workspace` (not just the target binary) before merging. CI catches this but it should never have shipped. Signed-off-by: mik-tf
mik-tf commented 2026-04-27 04:53:56 +00:00
Author
Owner
Copy link

Fixed in hero_agent commit e876a16 on development. All three failure modes from the issue body addressed.

A. Naming regex — new tool_router::sanitize_tool_name(name):

pub fn sanitize_tool_name(name: &str) -> String {
    // Replace any char outside [a-zA-Z0-9_-] with `_`, collapse runs.
    let mut out = String::with_capacity(name.len());
    let mut prev_was_underscore = false;
    for ch in name.chars() {
        let ok = ch.is_ascii_alphanumeric() || ch == '_' || ch == '-';
        if ok {
            out.push(ch); prev_was_underscore = ch == '_';
        } else if !prev_was_underscore {
            out.push('_'); prev_was_underscore = true;
        }
    }
    while out.ends_with('_') && out.len() > 1 { out.pop(); }
    if out.len() > 128 { out.truncate(128); }
    out
}

Applied at every emission site:

  • tool_router::ToolRouter::build_schemas
  • agent::Agent::build_schemas_for_names

Guarantees output matches ^[a-zA-Z0-9_-]{1,128}$ (regression test asserts against a bad-input matrix).

Examples:

Input Output
hero_osis.contact.list hero_osis_contact_list
rpc.discover rpc_discover
foo..bar (runs collapse) foo_bar
hero_osis. (trailing trim) hero_osis
200 a chars (truncate) 128 a chars

B. Tool count cap + duplicate dedup — new tool_router::finalize_schemas(schemas, cap):

pub fn finalize_schemas(schemas: Vec<ToolSchema>, cap: usize) -> Vec<ToolSchema> {
    let mut seen: HashSet<String> = HashSet::new();
    let mut deduped: Vec<ToolSchema> = Vec::with_capacity(schemas.len());
    for s in schemas {
        if seen.insert(s.function.name.clone()) {
            deduped.push(s);
        } else {
            tracing::warn!(
                tool = %s.function.name,
                "duplicate tool name after sanitization — second registration dropped (home#153)"
            );
        }
    }
    deduped.truncate(cap);
    deduped
}

Wired into both build_schemas paths so every tool list emitted to an LLM is at most 128 entries with no duplicates. Collisions warn-and-drop (the issue body's requirement: "naming collisions should be reported, not silently dropped").

C. Dispatch reverse lookup — new tool_router::mcp_tool_for_sanitized(name, mcp_tools):

pub fn mcp_tool_for_sanitized<'a>(
    sanitized: &str,
    mcp_tools: &'a [&'a McpTool],
) -> Option<&'a McpTool> {
    mcp_tools
        .iter()
        .find(|t| sanitize_tool_name(&t.name) == sanitized)
        .copied()
}

In agent::Agent::agent_loop, when the LLM echoes back a sanitized tool name, agent.rs reverse-looks-up the original McpTool.name (with dots) before calling mcp.call_tool. Falls back to the literal name if no match (e.g. tool cache changed mid-conversation, or it was a built-in tool — sanitization is a no-op for snake_case names).

Verification:

10 new tests:

  • sanitize_replaces_dots_with_underscores
  • sanitize_passes_already_clean_names
  • sanitize_collapses_runs_of_invalid_chars
  • sanitize_trims_trailing_underscores
  • sanitize_truncates_at_128_bytes
  • sanitize_matches_anthropic_regex — regression: every output must match the regex against a representative bad-input set
  • mcp_tool_for_sanitized_finds_original
  • mcp_tool_for_sanitized_returns_none_for_unknown
  • finalize_dedupes_by_name
  • finalize_caps_at_provider_limit

cargo fmt --check, cargo check -p hero_agent, cargo clippy --all-targets -- -D warnings all clean. All 107 hero_agent lib tests pass (97 pre-existing + 10 new).

Note on the issue body's medium-/long-term suggestions:

  • Per-call tool selection (4) — already in place via the existing mcp_groups system; this fix doesn't change that path.
  • Adapter-specific payloads (5) — deferred. The current tool_choice + sanitization handles every supported provider's hard constraints; adapter-specific tuning (e.g. Gemini-only quirks beyond name sanitization) can land separately when a specific incompat surfaces.
  • MCP namespacing in hero_router (6) — long-term ideal. With this fix, the existing dot-namespaced names work, so the hero_router refactor is no longer urgent.

Meta-tracker: home#193.

Signed-off-by: mik-tf

Fixed in hero_agent commit `e876a16` on `development`. All three failure modes from the issue body addressed. **A. Naming regex** — new `tool_router::sanitize_tool_name(name)`: ```rust pub fn sanitize_tool_name(name: &str) -> String { // Replace any char outside [a-zA-Z0-9_-] with `_`, collapse runs. let mut out = String::with_capacity(name.len()); let mut prev_was_underscore = false; for ch in name.chars() { let ok = ch.is_ascii_alphanumeric() || ch == '_' || ch == '-'; if ok { out.push(ch); prev_was_underscore = ch == '_'; } else if !prev_was_underscore { out.push('_'); prev_was_underscore = true; } } while out.ends_with('_') && out.len() > 1 { out.pop(); } if out.len() > 128 { out.truncate(128); } out } ``` Applied at every emission site: - `tool_router::ToolRouter::build_schemas` - `agent::Agent::build_schemas_for_names` Guarantees output matches `^[a-zA-Z0-9_-]{1,128}$` (regression test asserts against a bad-input matrix). Examples: | Input | Output | |-------|--------| | `hero_osis.contact.list` | `hero_osis_contact_list` | | `rpc.discover` | `rpc_discover` | | `foo..bar` (runs collapse) | `foo_bar` | | `hero_osis.` (trailing trim) | `hero_osis` | | 200 `a` chars (truncate) | 128 `a` chars | **B. Tool count cap + duplicate dedup** — new `tool_router::finalize_schemas(schemas, cap)`: ```rust pub fn finalize_schemas(schemas: Vec<ToolSchema>, cap: usize) -> Vec<ToolSchema> { let mut seen: HashSet<String> = HashSet::new(); let mut deduped: Vec<ToolSchema> = Vec::with_capacity(schemas.len()); for s in schemas { if seen.insert(s.function.name.clone()) { deduped.push(s); } else { tracing::warn!( tool = %s.function.name, "duplicate tool name after sanitization — second registration dropped (home#153)" ); } } deduped.truncate(cap); deduped } ``` Wired into both `build_schemas` paths so every tool list emitted to an LLM is at most 128 entries with no duplicates. Collisions warn-and-drop (the issue body's requirement: "naming collisions should be reported, not silently dropped"). **C. Dispatch reverse lookup** — new `tool_router::mcp_tool_for_sanitized(name, mcp_tools)`: ```rust pub fn mcp_tool_for_sanitized<'a>( sanitized: &str, mcp_tools: &'a [&'a McpTool], ) -> Option<&'a McpTool> { mcp_tools .iter() .find(|t| sanitize_tool_name(&t.name) == sanitized) .copied() } ``` In `agent::Agent::agent_loop`, when the LLM echoes back a sanitized tool name, agent.rs reverse-looks-up the original `McpTool.name` (with dots) before calling `mcp.call_tool`. Falls back to the literal name if no match (e.g. tool cache changed mid-conversation, or it was a built-in tool — sanitization is a no-op for snake_case names). **Verification:** 10 new tests: - `sanitize_replaces_dots_with_underscores` - `sanitize_passes_already_clean_names` - `sanitize_collapses_runs_of_invalid_chars` - `sanitize_trims_trailing_underscores` - `sanitize_truncates_at_128_bytes` - `sanitize_matches_anthropic_regex` — regression: every output must match the regex against a representative bad-input set - `mcp_tool_for_sanitized_finds_original` - `mcp_tool_for_sanitized_returns_none_for_unknown` - `finalize_dedupes_by_name` - `finalize_caps_at_provider_limit` `cargo fmt --check`, `cargo check -p hero_agent`, `cargo clippy --all-targets -- -D warnings` all clean. All 107 hero_agent lib tests pass (97 pre-existing + 10 new). **Note on the issue body's medium-/long-term suggestions:** - Per-call tool selection (4) — already in place via the existing `mcp_groups` system; this fix doesn't change that path. - Adapter-specific payloads (5) — deferred. The current `tool_choice` + sanitization handles every supported provider's hard constraints; adapter-specific tuning (e.g. Gemini-only quirks beyond name sanitization) can land separately when a specific incompat surfaces. - MCP namespacing in hero_router (6) — long-term ideal. With this fix, the existing dot-namespaced names work, so the hero_router refactor is no longer urgent. Meta-tracker: home#193. Signed-off-by: mik-tf
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
development
main
No results found.
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
lhumina_code/home#153
No description provided.