luigidemasi commented on code in PR #27480:
URL: https://github.com/apache/camel/pull/27480#discussion_r4204879642
##########
components/camel-ai/camel-ai-tool/src/main/java/org/apache/camel/component/ai/tool/AiToolExecutor.java:
##########
@@ -176,17 +177,42 @@ public static AiToolResult execute(AiToolSpec spec,
Map<String, Object> argument
* present in its cause chain (the route's {@link
org.apache.camel.spi.AuthorizationPolicy} rejected the call).
* Returns a caller-safe {@link AiToolResult.AuthorizationDenied} refusal
that does not leak the policy's internal
* message, or {@code null} when the error is not an authorization denial.
+ * <p>
+ * On a denial it also logs at {@code WARN} and emits an {@link
AiToolAuthorizationDeniedEvent} so operators can
+ * observe and alert on denials; neither affects the refusal returned to
the model.
*/
- private static AiToolResult authorizationDenied(String toolName, Throwable
error) {
+ private static AiToolResult authorizationDenied(String toolName, Throwable
error, Exchange exchange) {
CamelAuthorizationException denial = findAuthorizationException(error);
if (denial == null) {
return null;
}
LOG.warn("Tool '{}' call denied by authorization policy: {}",
toolName, denial.getMessage());
+ fireAuthorizationDeniedEvent(exchange, toolName, denial);
return new AiToolResult.AuthorizationDenied(
String.format("Access denied: not authorized to call tool
'%s'", toolName), denial);
}
+ /**
+ * Emits an {@link AiToolAuthorizationDeniedEvent} for the denied tool
call. The policy guards the route's outer
+ * processor, so a denial never runs the route's unit of work and fires no
exchange-lifecycle event or route span;
+ * this event is the observable signal. Best-effort: when no {@code
EventNotifier} is registered nothing is emitted,
+ * and a failure to notify is swallowed so it never affects the refusal
returned to the model.
+ */
+ private static void fireAuthorizationDeniedEvent(Exchange exchange, String
toolName, CamelAuthorizationException denial) {
+ ManagementStrategy management =
exchange.getContext().getManagementStrategy();
+ if (management.getEventNotifiers().isEmpty()) {
+ return;
+ }
+ try {
+ management.notify(new AiToolAuthorizationDeniedEvent(exchange,
toolName, denial));
+ } catch (Exception e) {
Review Comment:
**P2 — Preserve the authorization refusal when a notifier throws an Error.**
This catches only `Exception`, and `DefaultManagementStrategy.notify()` does
not isolate failures from individual notifiers. If a registered notifier throws
an `AssertionError` while handling this event, that error escapes
`AiToolExecutor.execute()` instead of returning `AuthorizationDenied`. I
reproduced this with a denying policy and a notifier that throws only for
`AiToolAuthorizationDeniedEvent`: the parent revision returns
`AuthorizationDenied`, while this revision propagates the assertion error.
That contradicts the new guarantee that notification failures never affect
the refusal returned to the model. Please catch `Throwable` here, consistent
with `EventHelper.doNotifyEvent()`, and add a regression test proving the
refusal survives a notifier failure. The guarded route remains blocked; the
regression is in refusal handling.
_AI-generated review by Codex on behalf of Luigi De Masi (@luigidemasi)._
--
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]
