gnodet commented on PR #24992:
URL: https://github.com/apache/camel/pull/24992#issuecomment-5047995482
_Claude Code on behalf of gnodet_
@oscerd Thank you for the thorough review — the analysis of the
post-CAMEL-23382 state was exactly right. Here's what I've done in the rebase:
### Rebased onto current `main`
The branch is now based on `main` after CAMEL-23382. The fix targets the new
`AiToolExecutor` code path:
- **`addToolToResult()`** — Added `ExchangeHelper.createCopy(exchange,
true)` before `AiToolExecutor.execute()` to isolate each tool invocation
(fixing bug 3: shared exchange)
- **`toToolResponse()`** — Changed `ExecutionError` handling from returning
`"Tool execution failed"` to `throw
RuntimeCamelException.wrapRuntimeCamelException(error.cause())` (fixing bug 2:
exception swallowing)
### Addressed review feedback
1. **`RuntimeCamelException` instead of `RuntimeException`** ✅ — Using
`RuntimeCamelException.wrapRuntimeCamelException()` so downstream
`ToolExecutionErrorHandler.handle(Throwable, ToolErrorContext)` can type-match
the real cause without unwrapping.
2. **Fixed `toolExecutionShouldNotLeakStateIntoCallingExchange`** ✅ — The
test now:
- Sends with a known body and `originalHeader=originalValue`
- Passes actual tool arguments (`{"input": "test value"}`) so
`AiToolExecutor` sets argument headers
- Tool route sets a `toolSideEffect` header for route-level leakage
detection
- Asserts: original header survives, `input` argument header is absent,
`toolSideEffect` header is absent
- Added a separate
`failingToolExecutionShouldNotLeakStateIntoCallingExchange` for the failure path
3. **Tool URIs migrated** — All test routes use `ai-tool:` instead of
`langchain4j-tools:`
### Bug 1 (missing exchange.getException() check)
Confirmed already fixed in `AiToolExecutor.execute()` on `main` — no action
needed.
### Re: upgrade guide entry
Good call on the upgrade guide. The exchange isolation change is
user-visible (tool-route header/body mutations and tool argument headers will
stop propagating to the calling exchange). I'll add an upgrade guide entry once
the fix is confirmed by CI.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
