testing ci on rebased branch - #177
Closed
chrishagglund-ship-it wants to merge 5 commits into
Closed
Conversation
_extractToolCalls named every call after its task type, so HTTP, MCP and
human tools came back as "http", "call_mcp_tool" and "human". The line
that was meant to prevent that read inputData.method five lines after
INTERNAL_KEYS had deleted it, so the ?? fallback to taskType was
unconditional. It also case-folded, reporting a tool named getWeather as
"getweather".
Two narrower defects had the same blast radius:
- SUB_WORKFLOW sat on the skip list, and an agent exposed as a tool
compiles to one, so agent-as-tool calls were dropped entirely.
- Selection required a reference name starting with "call_", which is
OpenAI's tool-call id format. Anthropic emits "toolu_" and a blank id
becomes a UUID, so an Anthropic-backed agent recorded no tool calls
at all. Adding "toolu_" only defers the problem to the next provider.
Select on inputData._agent_tool_name instead, which the server's dispatch
script writes on every dispatched tool and which no orchestration task
carries. It is also the only name that holds across tool kinds: the task
type is the transport, and taskDefName is the transport's own name for
MCP (call_mcp_tool), agent (the sub-workflow's name) and media tools.
The marker is stripped from the reported args alongside the other
internal keys.
Tasks without the marker — servers older than it — keep the previous
"call_" heuristic, but take their name from taskDefName rather than the
task type.
result.events is unaffected; it is populated from the stream's own
accumulator and was already correct.
_agent_tool_name is written by the dispatch script agents compile to when their tools are declared up front. Agents that discover tools at runtime compile to a second script (enrichToolsScriptDynamic), which builds the same tasks and never writes the marker — so on that path the previous commit fell straight through to the "call_" reference-name heuristic and an Anthropic-backed MCP agent still recorded nothing. Selecting purely by task type, as one reading of the bug report suggests, is not available: the agent compiler emits 26 SIMPLE, 8 SUB_WORKFLOW and a HUMAN task of its own for guardrail workers, handoffs and approvals, so those types cannot distinguish a tool call from scaffolding. HTTP and CALL_MCP_TOOL can. Neither appears anywhere in the compiled agent outside tool dispatch, so a task of either type is a tool call whether or not it carries the marker. CALL_MCP_TOOL names the tool in `method` — the field the original code meant to read before the internal- key strip removed it — and that read stays scoped to CALL_MCP_TOOL so a worker tool taking its own `method` argument is not renamed by it. Unmarked SIMPLE, SUB_WORKFLOW and HUMAN tools keep the "call_" heuristic and are still missed on a non-OpenAI provider. Closing that needs the marker on the runtime-discovery path, which is a server-side change.
Verified against a live 5.5.0 server: an agent tool's _agent_tool_name
does not stay at the top of inputData. The sub-workflow task mapper
rebuilds inputData around workflowInput and the marker rides along
inside it, so the previous commit fell through to the "call_" heuristic
and an agent-as-tool was still missed on a non-OpenAI provider.
A handoff compiles to SUB_WORKFLOW too and carries the marker at neither
level -- confirmed on a real handoff run, whose router and handoff tasks
both come back unmarked. That absence is what keeps the two apart, so
reading the nested marker does not turn handoffs into tool calls.
Also stop upper-casing the last-resort name. The task-type fallback fed
on the already-upper-cased copy, so a worker tool reached through it was
reported as GETWEATHER -- louder than main's getweather and no more
correct. A SIMPLE task's type is the tool's own name, so the raw
spelling is the one worth keeping.
The trimmed task shape run() actually receives is now pinned by tests.
GET /agent/execution/{id} returns taskType, referenceTaskName, status
and outputData with no inputData and no taskDefName, so no marker
reaches the extraction on that path at all.
Say what the code does and why, not how; the fallback order is already legible in the code itself.
The comment said unmarked tasks fall back to the task type; they fall back to taskDefName. _nonEmptyString says what it checks and that it can return undefined.
Contributor
Author
|
confirmed s.b. good. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.