CAMEL-23397: Support route tool annotations (returnDirect) in camel-openai agentic loop - #25497
Conversation
Bugbot re-review follow-upAI-generated on behalf of atiaomar1978-hub using Cursor. After the initial bugbot review, the following issues were addressed in commits Fixed
Tests re-run./mvnw -pl components/camel-ai/camel-openai -am test \
-Dtest=OpenAIRouteToolSupportTest,OpenAIRouteToolReturnDirectTest,McpToolCallExecutorTest,OpenAIEndpointMcpToolRefreshTestAll passed locally. |
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
…-openai Add returnDirect to ai-tool annotations/configuration and publish it via the MCP server bridge. Wire camel-openai to the shared AiToolRegistry through a tags option so route-based tools are exposed alongside MCP tools, executed via AiToolExecutor, and honour returnDirect in the agentic loop. Co-authored-by: Cursor <cursoragent@cursor.com>
Keep MCP tools and returnDirect flags when route tools are refreshed or removed, dedupe shadowed MCP tools when a route tool shares the same name, and allow manual tool-execution loops with tags-only route tools. Co-authored-by: Cursor <cursoragent@cursor.com>
Run republishCombinedState after MCP reconnect and tool-list refresh so route tool shadowing and returnDirect merging stay consistent. Co-authored-by: Cursor <cursoragent@cursor.com>
799b8ae to
7f4506e
Compare
|
There are uncommitted changes |
Commit generated ai-tool and openai catalog, documentation, and DSL artifacts so CI sourcecheck passes after adding returnDirect and tags options. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Fixed CI uncommitted changes failure in run 31782070449. Regenerated and committed missing catalog/DSL artifacts for
Commit: AI-generated on behalf of atiaomar1978-hub |
davsclaus
left a comment
davsclaus
left a comment
There was a problem hiding this comment.
Claude Code on behalf of davsclaus
Thanks for the contribution — the feature design is solid and the test coverage is good. A few issues to address before this can merge.
Findings
1. AiToolAnnotations record constructor is public API — adding a field is a binary-breaking change (High)
AiToolAnnotations is a public record (since 4.22) in camel-ai-tool. Adding returnDirect as a 6th constructor parameter breaks any downstream code constructing new AiToolAnnotations(title, readOnly, destructive, idempotent, openWorld) — that 5-arg call no longer compiles. Per project conventions: "Do NOT change public API signatures without justification." If 4.22 has not shipped yet this is harmless (pre-release), but please confirm and note it in the PR description. If it has shipped, a secondary 5-arg compat constructor is needed.
2. FQCNs used in McpToolState.knownToolNames() (Medium)
The new method uses fully-qualified class names inline (java.util.Set, java.util.HashSet). Per project conventions: "Do NOT use fully qualified class names in Java code. Always add an import statement and use the simple class name." Both Set and HashSet are already imported at the top of the file — just use the simple names.
3. Dead release variable in McpToolCallExecutor.executeRouteTool() (Medium)
boolean release = true; is set once and never changed to false, making the if (release) branch unconditional. Remove the variable and call releaseExchange directly in the finally block.
4. New compile dependency camel-ai-tool → camel-openai — needs justification (Low)
Per project conventions: "Do NOT add new dependencies without justification." This coupling is architecturally reasonable, but please add a brief note in the PR description explaining why camel-openai needs a compile dependency on camel-ai-tool rather than going through an SPI.
5. Commit co-authorship attribution
All 4 commits show cursoragent as the author. Per project AI contribution rules: "commits have proper co-authorship attribution (e.g., Co-authored-by trailers)." The PR description mentions Cursor, but the individual commits should carry the trailer.
Note: CI checks are still pending at time of review. This review does not replace specialized AI review tools (CodeRabbit, Sourcery) or static analyzers (SonarCloud).
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
|
|
||
| static McpToolState empty() { | ||
| return new McpToolState(List.of(), Map.of(), Map.of(), Set.of()); | ||
| return new McpToolState(List.of(), Map.of(), Map.of(), Set.of(), Map.of()); |
There was a problem hiding this comment.
These use fully-qualified class names (java.util.Set, java.util.HashSet) but both are already imported at the top of this file. Per project conventions, use the simple class names:
| return new McpToolState(List.of(), Map.of(), Map.of(), Set.of(), Map.of()); | |
| Set<String> knownToolNames() { | |
| Set<String> names = new HashSet<>(toolClientMap.keySet()); |
|
|
||
| try { | ||
| Map<String, Object> argsMap = OBJECT_MAPPER.readValue(argsJson, Map.class); | ||
| Exchange toolExchange = spec.getConsumer().getEndpoint().createExchange(); |
There was a problem hiding this comment.
release is set to true and never changed — the conditional is always entered. Remove the variable and call releaseExchange unconditionally in the finally block.
| Exchange toolExchange = spec.getConsumer().getEndpoint().createExchange(); | |
| boolean release = true; |
Should become just removing the variable and changing if (release) { to unconditional.
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 17 tested, 26 compile-only — current: 14 all testedMaveniverse Scalpel detected 43 affected modules (current approach: 14).
|
3 participants
Description
AI-generated on behalf of atiaomar1978-hub using Cursor.
Implements CAMEL-23397: parity between MCP tools and route-based
ai-tooltools in thecamel-openaiagentic loop.Changes
camel-ai-tool: addreturnDirecttoAiToolAnnotations/AiToolConfigurationand document it.camel-mcp-server: publishreturnDirectin MCP tool annotations viaVertxMcpServerEngine.camel-openai:tagsendpoint option to discover route tools from the sharedAiToolRegistryAiToolSpecToOpenAI,OpenAIRouteToolSupport)McpToolCallExecutorviaAiToolExecutorreturnDirectannotations when short-circuiting the agentic looptool-executionloops with tags-only route toolsTesting
./mvnw -pl components/camel-ai/camel-ai-tool,components/camel-ai/camel-openai -am test \ -Dtest=AiToolAnnotationsTest,OpenAIRouteToolSupportTest,OpenAIRouteToolReturnDirectTest,McpToolCallExecutorTest,OpenAIProducerMcpMockTest,OpenAIEndpointMcpToolRefreshTestTarget
mainbranch)Tracking
Apache Camel coding standards and style
mvn clean install -DskipTestslocally from root folder and I have committed all auto-generated changes.AI-assisted contributions
Co-authored-bytrailers) and the PR description identifies the AI tool used.