Ashfaqbs commented on code in PR #1071:
URL: https://github.com/apache/flink-agents/pull/1071#discussion_r3892237004
##########
runtime/src/main/java/org/apache/flink/agents/runtime/context/RunnerContextImpl.java:
##########
@@ -576,6 +576,12 @@ protected <T> T durableExecuteCompletionOnly(
Exception exception = null;
try {
result = executionCallable.call();
+ } catch (InterruptedException e) {
+ // A cancellation signal, not a genuine call failure: leave the
durable slot
+ // unfinished so recovery re-executes or reconciles the call
instead of replaying a
+ // stale interruption as a completed success or failure.
+ Thread.currentThread().interrupt();
+ throw e;
Review Comment:
Traced it through — matches what you described. It's real but needs a
RoutingStrategy that does I/O, which nothing in-tree does today, so it's about
the extension point rather than this PR's own code. Given the issue is scoped
to the chat/tool call paths, I'd rather keep this PR to what it already touches
and file a follow-up for the routing-resolver + IGNORE interaction rather than
pull it in here — let me know if you'd rather it go in this PR instead.
##########
runtime/src/main/java/org/apache/flink/agents/runtime/context/RunnerContextImpl.java:
##########
@@ -939,6 +945,12 @@ protected <T> T executeAndFinalizeCurrentCall(
Exception exception = null;
try {
result = callSupplier.call();
+ } catch (InterruptedException e) {
+ // A cancellation signal, not a genuine call failure: leave the
pending call
+ // unfinalized so recovery re-executes or reconciles it instead of
replaying a stale
+ // interruption as a completed success or failure.
+ Thread.currentThread().interrupt();
+ throw e;
Review Comment:
Confirmed, same shape in both executeSequentially and executeParallel, and
it predates this PR. Tool-call cancellation handling looks like its own piece
of work (the response still gets sent and the action still finishes), not a
natural extension of the chat-retry fix here. I'd lean toward a separate
issue/PR for it rather than scope-creeping this one — open to doing it here
instead if you'd prefer to keep it together.
--
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]
