[AI] fix: ignore tool calls from aborted assistant turns#3223
[AI] fix: ignore tool calls from aborted assistant turns#3223
Conversation
| it("ignores tool calls from aborted assistant messages", () => { | ||
| const sm = SessionManager.inMemory(); | ||
| installSessionToolResultGuard(sm); | ||
|
|
||
| sm.appendMessage({ | ||
| role: "assistant", | ||
| stopReason: "aborted", | ||
| content: [{ type: "toolCall", id: "call_abort", name: "read", arguments: {} }], | ||
| } as AgentMessage); | ||
| sm.appendMessage({ | ||
| role: "assistant", | ||
| content: [{ type: "text", text: "next" }], | ||
| } as AgentMessage); | ||
|
|
||
| const messages = sm | ||
| .getEntries() | ||
| .filter((e) => e.type === "message") | ||
| .map((e) => (e as { message: AgentMessage }).message); | ||
|
|
||
| expect(messages.map((m) => m.role)).toEqual(["assistant", "assistant"]); | ||
| }); |
There was a problem hiding this comment.
[P2] Test doesn’t assert the main failure mode (no synthetic toolResult is inserted)
This test only checks roles are ["assistant", "assistant"], but it would still pass if a synthetic toolResult were appended and then later filtered out elsewhere. To make the regression tighter, consider also asserting that guard.getPendingIds() is empty and/or that no message with role === "toolResult" exists in the transcript after appending the aborted toolCall + next assistant message.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agents/session-tool-result-guard.test.ts
Line: 55:75
Comment:
[P2] Test doesn’t assert the main failure mode (no synthetic toolResult is inserted)
This test only checks roles are `["assistant", "assistant"]`, but it would still pass if a synthetic toolResult were appended and then later filtered out elsewhere. To make the regression tighter, consider also asserting that `guard.getPendingIds()` is empty and/or that no message with `role === "toolResult"` exists in the transcript after appending the aborted toolCall + next assistant message.
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.| if [ -n "$1" ]; then | ||
| MASTER_PW="$1" | ||
| else | ||
| read -sp "Bitwarden master password: " MASTER_PW | ||
| echo | ||
| fi |
There was a problem hiding this comment.
[P3] Helper script encourages passing master password as a CLI arg
source bw-session.sh <master_password> (and $1) exposes the master password via shell history and ps output on many systems. Even if this is just a local helper, it’s safer to only use the interactive read -sp path (or accept the password via stdin) and update the usage text accordingly.
Prompt To Fix With AI
This is a comment left during a code review.
Path: skills/bitwarden/scripts/bw-session.sh
Line: 8:13
Comment:
[P3] Helper script encourages passing master password as a CLI arg
`source bw-session.sh <master_password>` (and `$1`) exposes the master password via shell history and `ps` output on many systems. Even if this is just a local helper, it’s safer to only use the interactive `read -sp` path (or accept the password via stdin) and update the usage text accordingly.
How can I resolve this? If you propose a fix, please make it concise.
Summary
What's the problem?
When an assistant turn is aborted, it can still contain toolCall blocks. Our guard treats those as real tool calls and may synthesize toolResult entries. That creates toolResults with no corresponding toolUse in the immediately previous assistant turn, which strict providers (e.g. Anthropic-compatible APIs) reject.
How do I reproduce it?
LLM request rejected: ... unexpected tool_use_id found in tool_result blocks ... Each tool_result block must have a corresponding tool_use block in the previous message.Fix
Treat aborted assistant messages as having no tool calls. This prevents synthetic toolResults from being inserted for those aborted calls.
Testing
AI Assistance
Greptile Overview
Greptile Summary
This PR prevents session transcript “tool call repair” logic from treating aborted assistant turns as real tool calls.
Concretely:
extractAssistantToolCallsinsrc/agents/session-tool-result-guard.tsnow returns no tool calls whenstopReason === "aborted", which avoids synthesizing toolResult messages for tool calls that never actually executed.src/agents/session-transcript-repair.tsso transcript repair won’t reintroduce orphan toolResults for aborted turns.src/agents/session-tool-result-guard.test.tscovers the aborted-turn scenario.This fits the existing strict-provider compatibility goal of keeping
toolResultblocks paired immediately after their corresponding tool-use turn, preventing request rejection by providers that enforce this invariant.Confidence Score: 4/5
stopReason === "aborted"in two tool-call extractors) with a regression test. Main residual risk is behavioral: if some upstream emits aborted turns where tool calls should be considered, this will now suppress them, but that seems aligned with the described issue.