Skip to content

CAMEL-23873: Introduce shared ToolRegistry for AI tools#24337

Merged
gnodet merged 1 commit into
apache:mainfrom
gnodet:the-new-camel-tui-is-an-entry-point-for-in-depth-k
Jul 2, 2026
Merged

CAMEL-23873: Introduce shared ToolRegistry for AI tools#24337
gnodet merged 1 commit into
apache:mainfrom
gnodet:the-new-camel-tui-is-an-entry-point-for-in-depth-k

Conversation

@gnodet

@gnodet gnodet commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Claude Code on behalf of Guillaume Nodet

Introduce a shared ToolRegistry in camel-jbang-core that serves as the single source of truth for AI tool definitions and execution logic, eliminating duplication between the MCP server and Agent REPL.

JIRA: CAMEL-23873

What changed

  • ToolRegistry (camel-jbang-core): Central registry of 51 shared tool descriptors with ToolDescriptor (builder-style metadata + executor), ToolContext (shared execution context wrapping RuntimeHelper + CamelCatalog), and ToolExecutor functional interface.
  • AskTools (Agent REPL): Refactored to delegate to ToolRegistry for shared tools. CLI/file tools remain local. (995 → 478 lines)
  • RuntimeTools (MCP server): Refactored to thin MCP wrappers delegating to ToolRegistry via delegateToRegistry(). Only camel_runtime_processes and camel_runtime_receive keep their own implementations.
  • New DevConsole tools: Circuit breakers, startup steps, datasources, spans, metrics — previously only available in the TUI, now accessible to AI assistants.
  • New analysis tools: Route analysis with anti-pattern hints, EIP stats, config drift detection.
  • New MCP prompts: camel_diagnose_route (7-step diagnosis workflow), camel_optimize_route (6-step optimization workflow).
  • Integrated tools from main: camel_runtime_heap_histogram (CAMEL-23870) now delegates to ToolRegistry's get_heap_histogram.

Architecture

ToolRegistry (camel-jbang-core)
  └── 51 shared tool descriptors (ToolDescriptor + ToolExecutor)
       ├── AskTools (Agent REPL) — delegates for shared tools
       └── RuntimeTools (MCP server) — delegates via thin MCP wrappers

CatalogTools (camel-jbang-mcp) — stays separate (versioned catalog, typed records)
ExampleTools (camel-jbang-mcp) — stays separate (typed records, Docker info)

Behavioral note

The metric and external parameters in camel_runtime_route_topology now default to true when omitted (matching ToolRegistry defaults and prior MCP behavior). Previously the delegation refactor was silently flipping them to false when null.

Test plan

  • ToolRegistryTest — 16 tests covering registration, execution, and edge cases
  • MCP module — compiles and passes tests
  • Both modules pass formatting checks
  • Verified delegateToRegistry() correctly bridges ToolExecutionExceptionToolCallException

🤖 Generated with Claude Code

@gnodet gnodet requested review from davsclaus June 30, 2026 09:45
@github-actions

Copy link
Copy Markdown
Contributor

🌟 Thank you for your contribution to the Apache Camel project! 🌟
🤖 CI automation will test this PR automatically.

🐫 Apache Camel Committers, please review the following items:

  • First-time contributors require MANUAL approval for the GitHub Actions to run
  • You can use the command /component-test (camel-)component-name1 (camel-)component-name2.. to request a test from the test bot although they are normally detected and executed by CI.
  • You can label PRs using skip-tests and test-dependents to fine-tune the checks executed by this PR.
  • Build and test logs are available in the summary page. Only Apache Camel committers have access to the summary.

⚠️ Be careful when sharing logs. Review their contents before sharing them publicly.

@github-actions github-actions Bot added docs dsl and removed docs labels Jun 30, 2026
@github-actions

github-actions Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

🧪 CI tested the following changed modules:

  • dsl/camel-jbang/camel-jbang-core
  • dsl/camel-jbang/camel-jbang-mcp

⚠️ Some tests are disabled on GitHub Actions (@DisabledIfSystemProperty(named = "ci.env.name")) and require manual verification:

  • dsl/camel-jbang/camel-jbang-core: 1 test(s) disabled on GitHub Actions
  • dsl/camel-jbang/camel-jbang-mcp: 1 test(s) disabled on GitHub Actions

💡 Manual integration tests recommended:

You modified dsl/camel-jbang/camel-jbang-core. The related integration tests in dsl/camel-jbang/camel-jbang-it are excluded from CI. Consider running them manually:

mvn verify -f dsl/camel-jbang/camel-jbang-it -Djbang-it-test
All tested modules (7 modules)
  • Camel :: JBang :: Core
  • Camel :: JBang :: MCP
  • Camel :: JBang :: Plugin :: MCP
  • Camel :: JBang :: Plugin :: Route Parser
  • Camel :: JBang :: Plugin :: TUI
  • Camel :: JBang :: Plugin :: Validate
  • Camel :: Launcher :: Container

⚙️ View full build and test results

@davsclaus davsclaus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: Shared ToolRegistry, DevConsole tools, analysis tools, and diagnostic prompts

This review evaluates the PR against project rules and conventions. It does not replace specialized AI review tools (CodeRabbit, Sourcery) or static analysis (SonarCloud).

Overall

Well-structured refactoring. The ToolRegistry pattern is clean, the registerStatusTool/registerRouteControlTool helpers eliminate boilerplate nicely, and backward compatibility is preserved (existing AskCliToolsTest tests pass, AskTools public API unchanged). The MCP prompts (camel_diagnose_route, camel_optimize_route) provide clear step-by-step workflows.

Findings

1. PR description overstates scope — MCP deduplication is not achieved yet (Medium)

The description claims this PR will "eliminate tool duplication between the MCP server and Agent REPL" and create a "single source of truth." However, the MCP server tools (RuntimeTools.java, CatalogTools.java, etc. in camel-jbang-mcp) are not refactored to use the new ToolRegistry — they retain their own Quarkus @Tool-annotated implementations backed by RuntimeService. Currently, ToolRegistry is consumed by AskTools only. This isn't a code defect, but the PR description should accurately reflect what was done vs. what's planned as follow-up.

2. Missing JIRA reference in commits (Low)

Per project conventions, commit format should be CAMEL-XXXX: Brief description. All 5 commits lack a JIRA reference.

3. sql_query tool should be marked destructive (Medium) — see inline comment

4. ToolContext.catalog() lazy init is not thread-safe (Low) — see inline comment

Questions

  1. Is refactoring the MCP server (RuntimeTools, CatalogTools, etc.) to consume ToolRegistry planned as a follow-up?
  2. The 5th commit says "Remove planning document now that implementation is complete" but no file removal appears in the diff. Was this a file added and removed within this branch?
  3. The DevConsole tools (get_startup_steps, get_datasources, get_spans, get_metrics) call actions/status sections that may not exist in all Camel applications. Has the behavior been verified for absent sections — does the IPC protocol return a clean "not available" response vs. an error?
  4. The PR body has unchecked manual test items ("Manual verification with a running Camel application" and "MCP server smoke test"). These should ideally be verified before merge.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

@gnodet gnodet requested a review from davsclaus June 30, 2026 12:45

@davsclaus davsclaus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated Review — PR #24337

Following up on my earlier review. The two new commits address all prior findings.

Prior findings — all addressed

  1. sql_query destructive flag — Now .readOnly(false).destructive(true). ✅
  2. ToolContext.catalog() thread safety — Proper double-checked locking with volatile fields. ✅
  3. MCP deduplicationRuntimeTools now delegates to ToolRegistry via delegateToRegistry(), with only camel_runtime_processes and camel_runtime_receive remaining as MCP-specific. The "single source of truth" claim is now accurate. ✅

New changes look good

  • get_memory enriched in ToolRegistry to combine memory/gc/threads sections — matches prior MCP behavior.
  • get_route_topology now has configurable metric/external params instead of hardcoded values.
  • trace_control throws on unknown action instead of silently passing through.
  • toJsonObject helper handles the String→JsonObject conversion cleanly with a reasonable fallback.
  • camel_runtime_receive properly documented as MCP-only.

Remaining (low severity, already flagged)

  • Commit messages lack JIRA references per project conventions (CAMEL-XXXX: ... format).

No blocking issues. The PR delivers on its stated goals — nice work consolidating the tool definitions.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

@gnodet gnodet marked this pull request as ready for review July 1, 2026 08:52
@gnodet gnodet force-pushed the the-new-camel-tui-is-an-entry-point-for-in-depth-k branch 2 times, most recently from ebc70d8 to 550620f Compare July 2, 2026 05:51
@gnodet gnodet changed the title Shared ToolRegistry, DevConsole tools, analysis tools, and diagnostic prompts CAMEL-23873: Introduce shared ToolRegistry for AI tools Jul 2, 2026
@gnodet gnodet requested a review from davsclaus July 2, 2026 05:51

@davsclaus davsclaus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good refactoring — the shared ToolRegistry cleanly eliminates duplication between AskTools (Agent REPL) and RuntimeTools (MCP server), with each caller handling protocol-specific concerns and delegating execution to the shared registry.

A couple of things to be aware of:

  1. Merge conflict with recent tools on main: Since this PR was branched, main gained camel_runtime_heap_histogram (PR #24364, CAMEL-23870) and camel_runtime_jfr_old_objects (CAMEL-23872) in RuntimeTools.java. These will need to be added to ToolRegistry when resolving the merge conflict.

  2. The new MCP prompts (camel_diagnose_route, camel_optimize_route) are well-structured guided workflows.

See inline comment for one behavioral change to verify.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Add a shared ToolRegistry in camel-jbang-core that serves as the single
source of truth for AI tool definitions and execution logic, eliminating
duplication between the MCP server and Agent REPL.

New classes:
- ToolRegistry: central registry of 50 shared tool descriptors with
  ToolDescriptor, ToolContext, and ToolExecutor functional interface
- ToolDescriptor: builder-style descriptor bundling metadata + executor
- ToolContext: shared execution context (RuntimeHelper + CamelCatalog)
- ToolExecutionException: unchecked exception for tool failures

Refactored consumers:
- AskTools (Agent REPL): delegates to ToolRegistry for shared tools,
  keeps CLI/file tools locally (995 -> 478 lines)
- RuntimeTools (MCP server): thin MCP wrappers delegating to ToolRegistry
  via delegateToRegistry() helper. Only camel_runtime_processes and
  camel_runtime_receive keep their own implementations.

New tools added to ToolRegistry:
- DevConsole: circuit breakers, startup steps, datasources, spans, metrics
- Analysis: route analysis with anti-pattern hints, EIP stats,
  config drift detection
- Catalog: dataformats, languages, EIP doc (previously MCP-only)

New MCP prompts:
- camel_diagnose_route: 7-step guided diagnosis workflow
- camel_optimize_route: 6-step guided optimization workflow

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@gnodet gnodet force-pushed the the-new-camel-tui-is-an-entry-point-for-in-depth-k branch from 550620f to 4d6add7 Compare July 2, 2026 06:59
@gnodet

gnodet commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

Claude Code on behalf of Guillaume Nodet

Thanks for the review! Both points addressed:

  1. Heap histogram integrated: Rebased on latest main and added get_heap_histogram to ToolRegistry. camel_runtime_heap_histogram in RuntimeTools now delegates to ToolRegistry like all other tools. (camel_runtime_jfr_old_objects doesn't appear to be on main yet — will integrate when it lands.)

  2. Topology defaults: Replied on the inline comment — the change from metric != null && metric to metric == null || metric is intentional to align with ToolRegistry defaults (true when omitted). Added a behavioral note in the PR description.

@gnodet gnodet requested a review from davsclaus July 2, 2026 06:59
@davsclaus

Copy link
Copy Markdown
Contributor

LGTM

@gnodet gnodet merged commit 95d5e75 into apache:main Jul 2, 2026
5 checks passed
@gnodet gnodet deleted the the-new-camel-tui-is-an-entry-point-for-in-depth-k branch July 2, 2026 07:43

@oscerd oscerd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this — the ToolRegistry consolidation itself is clean: it's populated once in a static{} block and exposed only through unmodifiable views, ToolContext is per-consumer (no global mutable state, so multi-context isolation holds), and it's well covered by ToolRegistryTest. No public-API break. 👍

One blocking issue before merge, though: the two new MCP prompts reference tool names that don't exist on the MCP surface.

Tool-name mismatch in camel_diagnose_route / camel_optimize_route (PromptDefinitions.java)

The MCP server exposes the runtime tools under their method names in RuntimeTools.javacamel_runtime_context, camel_runtime_routes, camel_runtime_health, camel_runtime_top, etc. The two new prompts instead instruct the model to call get_context, get_routes, get_health, get_top_processors, … — the ToolRegistry / AskTools (REPL) names, which are not registered as MCP @Tools.

Two consequences:

  1. The ~13 tools that do have an MCP equivalent are systematically misnamed — e.g. get_context → should be camel_runtime_context, get_top_processorscamel_runtime_top, get_route_dumpcamel_runtime_route_dump.
  2. Six referenced tools have no MCP counterpart at all: get_circuit_breakers, get_datasources, get_spans, get_metrics, get_route_analysis, get_eip_stats. These exist only in ToolRegistry for the REPL agent and are never surfaced as @Tool, so the "Advanced diagnostics" / infrastructure steps can't run when the prompt is used through the MCP server.

For reference, the existing sibling prompts in the same file (camel_build_integration, camel_migrate_project, camel_security_review) all use the fully-qualified camel_* names, so the two new prompts are the odd ones out.

Suggested fix: rename the ~13 to their camel_runtime_* equivalents, and for the 6 registry-only tools either expose them as MCP @Tools or drop those steps from the prompt text. A small test asserting that every tool name referenced in a @Prompt resolves to a registered MCP tool would prevent this class of drift (CI can't catch it today — the prompt bodies are just strings).

Minor (non-blocking)

  • Delegation changes MCP output/error shapes: via RuntimeTools.delegateToRegistry / toJsonObject, a "no status" case now returns {"result":"No status available…"} instead of throwing, and empty errors/history return {"result":…} instead of {"errors":[]}. Worth a note if any client relied on the old shapes.
  • camel_runtime_route_topology: metric / external now default to true when omitted (was false). It's mentioned in the PR description but not in camel-jbang-mcp.adoc.
  • AskTools.setTargetPid(-1) is guarded by if (targetPid >= 0), so resetting to a negative pid leaves ctx.pid pointing at the previously targeted process.

Reviewed with Claude Code on behalf of Andrea Cosentino. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants