fix(tool-loop): strict-model tool-call history (assistant+tool_calls / role:tool) — not a user-message fold#286
Merged
Conversation
…/ role:tool) — not a user-message fold
runToolLoop and streamToolLoop folded every tool outcome into a single
{ role: 'user', content: 'Tool results:\n…' } message and dropped the
assistant turn's tool_calls. A strict tool-history validator (Claude, and
any OpenAI-compatible provider that validates) can't see its own tool use
that way and re-issues the same call in a loop.
The loop now appends OpenAI function-calling shape: the assistant turn that
emitted the calls is preserved as an assistant message carrying its
tool_calls array (content null when the turn was tool-only), and each result
is its own { role: 'tool', tool_call_id, content } message keyed to its call.
A missing tool-call id is derived deterministically from the tool name so the
assistant entry and its result still match.
ToolLoopMessage widens additively: tool_calls / tool_call_id are optional and
content is string | null. A streamTurn that reads only role + content is
unaffected; one that forwards the whole message array to an OpenAI-compatible
endpoint now sends correct tool history.
tangletools
approved these changes
Jun 14, 2026
tangletools
left a comment
Contributor
There was a problem hiding this comment.
✅ Auto-approved PR — 687a35d3
Blanket team auto-approval is enabled for this reviewer service.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.
tangletools · auto-approval · reason: blanket_auto_approve · 2026-06-14T01:00:20Z
drewstone
added a commit
that referenced
this pull request
Jun 14, 2026
- feat(conversation): runPersonaConversation + runPersonaDispatch — the persona loop runner; any AgentProfile evaluated as a multi-round conversation, drops into runProfileMatrix as dispatch (#282) - feat(personify): connect the dormant analyst→steer wire + registryScopeAnalyst (#284) - feat(skills): build-with-agent-runtime canonical spine (#285) - fix(tool-loop): strict-model tool-call history (#286) - docs: canonical-api.md API reference (#283)
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
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.
The bug
runToolLoopandstreamToolLoop(the canonical turn-level tool-dispatch loop insrc/tool-loop.ts) folded every tool outcome into a single{ role: 'user', content: 'Tool results:\n…' }message and dropped the assistant turn'stool_calls. A strict tool-history validator — Claude, and any OpenAI-compatible provider that validates tool history — can't see its own tool use that way, so it re-issues the same tool call in a loop (until the stuck-loop backstop fires).ToolLoopMessagewas{ role: string; content: string }only, so the correct shape couldn't even be expressed.The fix
The loop now appends the OpenAI function-calling contract:
assistantmessage carrying itstool_calls[]array (contentisnullwhen the turn was tool-only);{ role: 'tool', tool_call_id, content }message keyed to the call that produced it;call_<toolName>) so the assistant entry and its result still match.Changes (
src/tool-loop.ts)ToolLoopMessagegains optionaltool_calls?: ToolLoopAssistantToolCall[]andtool_call_id?: string, andcontentwidens tostring | null. New exportedToolLoopAssistantToolCallinterface ({ id, type: 'function', function: { name, arguments } }). Added two helpers:assistantToolCallMessage(turnText, pending)andtoolResultMessage(call, content).runToolLoop(src/tool-loop.ts:218assistant push,:257budget-path tool result,:273per-result tool message) — replaces the conditional{ role: 'assistant', content: turnText }push + the terminal{ role: 'user', content: 'Tool results:…' }fold with oneassistant+tool_callsmessage then onerole: 'tool'message per result.streamToolLoop(src/tool-loop.ts:392assistant push,:437budget-path,:460per-result) — same replacement.Back-compat
This is additive. A
streamTurnthat reads onlyrole+contentis unaffected; one that forwards the whole message array to an OpenAI-compatible endpoint now sends correct tool history. Public function signatures (runToolLoop/streamToolLoop) are unchanged.Tests
New regression tests in
src/tool-loop.test.tscover both loops:assistant+tool_callsmessage then arole: 'tool'message per result, and norole: 'user'"Tool results" fold;call_submit_proposal) and matches across the assistant entry and its result (and tool-only turns carrycontent: null);Verified:
npx tsc --noEmitclean,pnpm run lintclean (the 2 remaining warnings are pre-existing, in an unrelated test file),pnpm testgreen (80 files / 880 passed / 1 skipped),pnpm run buildclean.Why now
This is the upstream of agent-app's local fix (its #20 /
ad2ceaa). agent-app currently carries its ownrunToolLoop/streamToolLoopbodies and cannot migrate onto the canonical loop until this lands. Once a release carrying this ships, agent-app collapses its hand-rolled loop to a thin re-export — a ~200-LOC delete.