fix(agent-harness): dedup visible tool specs in all provider-call paths

[YellowSnnowmann]

Summary

• Deduplicate ChatRequest.tools by name in the two remaining provider-call paths (run_tool_call_loop and subagent_runner::run_inner_loop); the Agent::turn() path was already deduped via rebuild_tool_policy_session() in Prioritize fully local speech and Composer operation #1710.
• Promote dedup_visible_tool_specs from pub(super) to pub(crate) and re-export it from agent::harness::session so all three call sites share one implementation and one set of tests.
• Add a regression test (run_tool_call_loop_dedups_duplicate_tool_names_before_provider_call) using a CapturingProvider that records ChatRequest.tools names and asserts duplicates are dropped before the provider sees them.
• Warn (tracing::warn!) in subagent_runner when duplicates are dropped, so future collisions are observable in production logs without re-triggering the Sentry 400.

Problem

Several upstream chat providers (notably OpenHuman's native chat-v1 route) reject requests whose tools array contains two entries with the same name, responding with HTTP 400 "Tool names must be unique". Sentry issue TAURI-RUST-4 captured 2,011 occurrences of this 400 from production. PR #1710 fixed the Agent::turn() path, but two adjacent paths were left exposed:

1. agent::harness::tool_loop::run_tool_call_loop — used by agent/bus.rs, channels, and CLI flows. Collisions occur when the same tool name appears in both tools_registry and the per-turn extra_tools list (e.g. dynamically synthesized variants).
2. agent::harness::subagent_runner::ops::run_inner_loop — used by typed sub-agents. Collisions occur when a parent-registry tool name shadows a dynamically-attached Composio action.

Either path could still emit ChatRequest.tools with duplicate names, surface the 400, and abort the turn.

Solution

• Lift the existing dedup_visible_tool_specs helper (already covered by 4 unit tests in session::builder::dedup_tests) to pub(crate) and re-export it via session::mod so sibling modules can reuse it without duplication.
• Replace the inline filter+collect in run_tool_call_loop with a dedup_visible_tool_specs(...) call. First-occurrence-wins semantics match the runtime tool-dispatch order (tools_registry.iter().chain(extra_tools.iter())), so the spec the model sees and the tool that actually executes stay aligned.
• In run_inner_loop, run dedup_visible_tool_specs over filtered_specs and emit a tracing::warn! when the length shrinks, including agent_id for correlation. allowed_names is built before dedup and is unaffected — every dropped entry's name is still present from its kept sibling.
• Design tradeoff: this introduces one extra Vec<ToolSpec> allocation per provider call (rather than retain-in-place), in exchange for a single shared helper with one set of tests. The allocation is negligible compared to the network call it precedes.

Submission Checklist

If a section does not apply to this change, mark the item as N/A with a one-line reason. Do not delete items.

• Tests added or updated (happy path + at least one failure / edge case) per Testing Strategy — new run_tool_call_loop_dedups_duplicate_tool_names_before_provider_call (failure path: pre-fix would have sent duplicates and 400'd); existing dedup_tests (4 cases) now cover all three call sites via the shared helper.
• Diff coverage ≥ 80% — changed lines (Vitest + cargo-llvm-cov merged via diff-cover) meet the gate enforced by .github/workflows/coverage.yml. Run pnpm test:coverage and pnpm test:rust locally; PRs below 80% on changed lines will not merge.
• Coverage matrix updated — N/A: behaviour-preserving fix; no new feature rows in [docs/TEST-COVERAGE-MATRIX.md](../docs/TEST-COVERAGE-MATRIX.md).
• All affected feature IDs from the matrix are listed in the PR description under ## Related — N/A: no matrix feature IDs touched.
• No new external network dependencies introduced (mock backend used per Testing Strategy) — test uses an in-process CapturingProvider.
• Manual smoke checklist updated if this touches release-cut surfaces (docs/RELEASE-MANUAL-SMOKE.md) — N/A: no user-visible surface change; behaviour is the absence of an HTTP 400.

Impact

• Runtime/platform: Rust core only. Affects every platform the desktop app runs on (macOS/Linux/Windows) and the standalone openhuman-core CLI binary, since both share the harness. No frontend/Tauri-shell changes.
• Performance: one additional Vec<ToolSpec> allocation + HashSet<String> walk per chat() call. O(n) over the per-turn tool count (typically < 50). Dwarfed by the provider round-trip it precedes.
• Security: none. Dedup uses the existing tool names already controlled by the harness; no new tool execution is enabled.
• Migration / compatibility: none. No persisted data, no protocol changes, no API surface change. dedup_visible_tool_specs visibility widens (pub(super) → pub(crate)) — strictly internal, does not affect the crate's public API or downstream consumers (Tauri shell verified via cargo check --manifest-path app/src-tauri/Cargo.toml).

Related

• Sentry: TAURI-RUST-4
• Prior art: Prioritize fully local speech and Composer operation #1710 (dedup in Agent::turn() via rebuild_tool_policy_session)

Summary by CodeRabbit

• Bug Fixes
  • Prevent duplicate tool specs from being sent to external providers by collapsing duplicates (first preserved); runtime/dynamic tools take precedence and duplicates log a warning.
• Tests
  • Added a regression test ensuring duplicate tool names are collapsed so providers receive a single entry.
• Localization
  • Added German translations for provider-unavailable messaging and MCP server configuration/settings text.

[coderabbitai]

Caution

Review failed

Failed to post review comments

📝 Walkthrough

Walkthrough

Widen and re-export dedup_visible_tool_specs, then apply it where harness code assembles visible ToolSpecs so duplicate tool names are removed (first occurrence kept) before provider calls; add a regression test capturing provider-observed tool names.

Changes

Tool spec deduplication across harness provider calls

Layer / File(s)	Summary
Visibility and module re-export src/openhuman/agent/harness/session/builder.rs, src/openhuman/agent/harness/session/mod.rs	dedup_visible_tool_specs visibility changed from pub(super) to pub(crate) and is re-exported from the session module.
Deduplication at provider calls src/openhuman/agent/harness/tool_loop.rs, src/openhuman/agent/harness/subagent_runner/ops.rs	run_tool_call_loop and run_typed_mode now collect visible ToolSpecs into an intermediate vector and call session::dedup_visible_tool_specs to drop duplicate tool names (keeping the first occurrence) before sending tools to providers.
Regression test for deduplication src/openhuman/agent/harness/tool_loop_tests.rs	Adds a CapturingProvider test helper and run_tool_call_loop_dedups_duplicate_tool_names_before_provider_call which asserts the provider sees each tool name only once when registry and extra_tools contain the same name.

German i18n additions

Layer / File(s)	Summary
German translation entries app/src/lib/i18n/chunks/de-3.ts, app/src/lib/i18n/chunks/de-5.ts	Adds subconscious.providerUnavailableTitle and subconscious.providerSettings plus multiple settings.mcpServer.* German translation keys and strings.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• tinyhumansai/openhuman#1858: Related work on using/re-exporting dedup_visible_tool_specs to de-duplicate visible tool specs before provider calls.

Suggested labels

working

Suggested reviewers

• senamakel

Poem

🐇 I found two echoes hopping in a row,
I left the first to speak and told the next to go.
Now the provider hears one clear chirp, not two,
Clean lists, calm chats—my job here is through.
Huzzah, tidy tools and fewer ado!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title 'fix(agent-harness): dedup visible tool specs in all provider-call paths' accurately and concisely summarizes the main change: deduplicating tool specs across multiple provider-call paths in the agent harness module to fix HTTP 400 errors.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/openhuman/agent/harness/subagent_runner/ops.rs`:
- Around line 841-855: The dedup currently keeps the first occurrence in
filtered_specs which preserves parent tool specs over extra_tools, but execution
prefers extra_tools; update the code so dedup_visible_tool_specs is given the
tool list in execution-precedence order (extra_tools before parent_tools) or
change dedup_visible_tool_specs to prefer later entries (i.e., keep the last
occurrence) so that extra_tools win on duplicate names; locate the use of
filtered_specs and the call to
crate::openhuman::agent::harness::session::dedup_visible_tool_specs and ensure
the order or dedup logic matches the runtime execution order referenced by
extra_tools and parent_tools.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9358788c-1649-46da-9927-c628b74fe055

📥 Commits

Reviewing files that changed from the base of the PR and between ec9708a and bb37212.

📒 Files selected for processing (5)

• src/openhuman/agent/harness/session/builder.rs
• src/openhuman/agent/harness/session/mod.rs
• src/openhuman/agent/harness/subagent_runner/ops.rs
• src/openhuman/agent/harness/tool_loop.rs
• src/openhuman/agent/harness/tool_loop_tests.rs

[graycyrus]

Clean fix for TAURI-RUST-4. Spec-build order matches dispatch order in both paths (tool_loop chains registry→extra, subagent_runner chains dynamic→parent), so dedup's first-occurrence-wins is correct. Shared helper avoids code duplication, regression test covers the collision scenario well. One minor nit inline.

File	Change
session/builder.rs	pub(super) → pub(crate) visibility bump
session/mod.rs	Re-export dedup_visible_tool_specs for sibling modules
tool_loop.rs	Dedup call before provider chat()
subagent_runner/ops.rs	Reorder specs to match execution precedence + dedup + warn
tool_loop_tests.rs	CapturingProvider regression test