Fix/deduplicate tool names

[Anubhav200311]

When multiple toolsets contain tools with the same name, the Agent's Tools() method returns duplicates. This causes Anthropic API 400 errors because the API rejects requests with duplicate tool names.

This fix adds deduplication logic to the Tools() method in pkg/agent/agent.go. After all toolsets are collected, it filters out duplicate tool names, keeping the first occurrence and logging a warning for
any that are skipped. A corresponding test case ("duplicate tool names are deduplicated") is added in pkg/agent/agent_test.go to verify the behavior.
Fixes #2251

[aheritier]

/review

[docker-agent]

Assessment: 🟡 NEEDS ATTENTION

This PR successfully fixes the duplicate tool name issue that was causing Anthropic API 400 errors. The deduplication logic is sound and will prevent the reported bug.

Findings: 1 medium-severity issue regarding inconsistent warning handling.

The deduplication warnings use slog.Warn() directly instead of addToolWarning(), which means they won't be captured by DrainWarnings(). Consider aligning with the existing warning pattern for consistency.

[Anubhav200311]

Addressed the bot's feedback in the latest commit --> added addToolWarning() for consistent warning handling.

[Anubhav200311]

Hi @aheritier , I've rebased onto main to clean up the commit history. Could you trigger a re-review when you get the time? Thanks!

[derekmisler]

/review

[aheritier]

/review

[docker-agent]

Assessment: 🟡 NEEDS ATTENTION

The deduplication logic is functionally correct and addresses the reported Anthropic API 400 error. The approach (first-occurrence wins, O(n) scan with a map) is clean and idiomatic Go.

One medium-severity issue was found in the new code, plus two low-severity observations:

Findings summary:

Severity	Finding
🟡 MEDIUM	Pre-existing pendingWarnings race extended by new dedup addToolWarning calls (see inline comment)
🔵 LOW	Empty-string tool names are silently deduplicated rather than flagged as invalid — minor edge case
🔵 LOW	Test verifies tool count and warning count but not which occurrence is retained (first vs. last) — a regression blind spot

Low-severity details:

• Empty tool name handling (pkg/agent/agent.go): If a tool has an empty Name, it will be treated as a duplicate of any other empty-named tool and silently discarded with a duplicate tool "": keeping first occurrence warning. This is likely a degenerate case, but a pre-check like if t.Name == "" { continue } with a distinct warning would surface configuration errors more clearly.

• Test coverage gap (pkg/agent/agent_test.go, line ~90): The test case checks wantToolCount: 3 and wantWarnings: 1, which confirms the count is right. It does not assert the identity of the retained tools (e.g., ["read_file", "write_file", "shell"] in that order). If the dedup logic were accidentally changed to keep the last occurrence, the test would still pass. Consider adding an assertion on the resulting tool names.

[docker-agent]

[MEDIUM/LIKELY] New addToolWarning call in dedup loop extends pre-existing unsynchronized access to pendingWarnings

a.addToolWarning(...) appends to a.pendingWarnings (a plain []string with no mutex). This pattern already existed in collectTools (line 235) and ensureToolSetsAreStarted (line 281) before this PR, but the new dedup block adds two additional unsynchronized append sites within the same hot path.

If two goroutines call Tools() or StartedTools() concurrently — which is plausible in a multi-agent or event-driven context — they both invoke collectTools, both reach this loop, and both call addToolWarning without synchronization, creating a data race on the backing array of the slice.

Impact: In practice the risk is low because agent methods are typically not called concurrently from different goroutines, and this is a pre-existing structural gap rather than a new one. However, the new dedup code is the first time duplicate-tool warnings can be generated during normal (non-error) operation, making this path more likely to be exercised.

Suggestion: Guard pendingWarnings with a sync.Mutex, or — if concurrent use is not a design goal — document that Tools() / StartedTools() must not be called concurrently.