Skip to content

feat: confirmToolCall support — startToolCall with requireConfirmation + ToolRunnableCallable [JAR-9208]#703

Open
JoshParkSJ wants to merge 4 commits intomainfrom
josh/cas-tc-fix
Open

feat: confirmToolCall support — startToolCall with requireConfirmation + ToolRunnableCallable [JAR-9208]#703
JoshParkSJ wants to merge 4 commits intomainfrom
josh/cas-tc-fix

Conversation

@JoshParkSJ
Copy link
Copy Markdown
Contributor

@JoshParkSJ JoshParkSJ commented Mar 17, 2026

Changes

  • Moves tool confirmation from the old interrupt-based flow to confirmToolCall on startToolCall:
  1. Graph build — LangGraph's wrapping discards the original BaseTool reference; the typed subclass preserves .tool so the runtime can inspect it at init time (so we know which tools need confirmation before hitting the tool node and can pass require confirmation on startToolCall)
  2. Runtime init_get_tool_confirmation_info walks the compiled graph once, finds ConversationalToolRunnableCallable nodes, and builds a {tool_name: input_schema} lookup. This is needed because coded agents (create_agent) export a compiled graph as the only artifact — there's no side channel to pass confirmation metadata from the build step to the runtime.
  3. Streaming — mapper checks the lookup on every AIMessage tool call, emits startToolCall with requireConfirmation: true and inputSchema.
  • fixes AIMessageChunk self-accumulation bug that doubled tool names on the first streamed chunk (only triggered with OpenAI, not Claude); startToolCall now always emitted for confirmation tools
  • rip-and-replace as we see zero production usage confirmed via App Insights telemetry (90 days)

Companion PRs

https://uipath.atlassian.net/browse/JAR-9208

@JoshParkSJ JoshParkSJ force-pushed the josh/cas-tc-fix branch 2 times, most recently from f129206 to 5700f6b Compare April 12, 2026 15:02
@JoshParkSJ JoshParkSJ force-pushed the josh/cas-tc-fix branch 6 times, most recently from 1eed1e2 to f655a29 Compare April 14, 2026 13:34
@JoshParkSJ JoshParkSJ changed the title fix: revert deferred tool call [JAR-9208] feat: confirmToolCall support — startToolCall with requireConfirmation + ToolRunnableCallable [JAR-9208] Apr 14, 2026
@JoshParkSJ JoshParkSJ force-pushed the josh/cas-tc-fix branch 9 times, most recently from 413c601 to 4d8b645 Compare April 15, 2026 02:01
Walks compiled graph nodes once at runtime init. This is needed because coded agents
(create_agent) export a compiled graph as the only artifact — there's no side channel
to pass confirmation metadata from the build step to the runtime.
"""
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

not a problem for low code agents, but might as well unify how we get tools requiring confirmation

Comment thread tests/runtime/test_chat_message_mapper.py Outdated
Comment on lines +343 to +349
# Skip the first chunk — it's already assigned as current_message above,
# so accumulating it with itself would duplicate fields via string concat
# (e.g. tool name "search_web" becomes "search_websearch_web").
if (
isinstance(self.current_message, AIMessageChunk)
and self.current_message is not message
):
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.

Is this what causes the duplicate tool name issue? I remember @shannonsuhendra wrote that this issue wasn't happening anymore.
If this change is risky maybe we can move to a different PR?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It happens only for GPT models. You can see it happen consistently in staging rn

Screenshot 2026-04-22 at 3 17 57 PM

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

updated the comment so it's clear this is for gpt only bug

def __init__(self, *, func: Any, afunc: Any, name: str, tool: BaseTool):
super().__init__(func=func, afunc=afunc, name=name)
self.tool = tool

Copy link
Copy Markdown
Contributor

@maxduu maxduu Apr 21, 2026

Choose a reason for hiding this comment

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

This part seems a little sketchy to me in terms of typing. It would be nice if the runtime.py code has the tool field typed, or at least can do an if isinstance ... to get the right class typing.
Some questions:

  • Any way we can add the fields needed by the tool to some metadata within RunnableCallable?
  • At the very least we don't need to do any for func, afunc in ConversationalToolRunnableCallable? But if we could do the first bullet it might be even better

Would be nice to get a look from @cristipufu when approach is finalized as well.

Copy link
Copy Markdown
Contributor Author

@JoshParkSJ JoshParkSJ Apr 25, 2026

Choose a reason for hiding this comment

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

Unfortunately RunnableCallable is a langgraph class, not ours to touch. Fixed func/afunc and if isintance ...

If you think ConversationalToolRunnableCallable is just ugly/excessive then we can force assign node.tool like this. Not sure if that's better though

    node = RunnableCallable(func=_func, afunc=_afunc, name=tool_name)

    tool = getattr(tool_node, "tool", None)
    if isinstance(tool, BaseTool):
        node.tool = tool  # type: ignore[attr-defined]

    return node

@maxduu maxduu self-requested a review April 21, 2026 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants