fix: drop assistant messages with stopReason 'error' to avoid orphaning tool results (#3860)#3880
Conversation
| // If the assistant message has an error stop reason, it might be filtered out | ||
| // by the provider on subsequent requests (e.g. Anthropic content filtering). | ||
| // We drop it to avoid orphaning any tool results that would be added below. | ||
| if ((assistant as any).stopReason === "error") { | ||
| changed = true; | ||
| continue; | ||
| } |
There was a problem hiding this comment.
[P2] Avoid as any for stopReason access
(assistant as any).stopReason bypasses type checking and is inconsistent with the rest of this file’s defensive-typing approach. Consider using a narrow structural cast (e.g. const stopReason = (assistant as { stopReason?: unknown }).stopReason; if (stopReason === "error") ...) so the check stays type-safe without introducing any.
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-transcript-repair.ts
Line: 120:126
Comment:
[P2] Avoid `as any` for `stopReason` access
`(assistant as any).stopReason` bypasses type checking and is inconsistent with the rest of this file’s defensive-typing approach. Consider using a narrow structural cast (e.g. `const stopReason = (assistant as { stopReason?: unknown }).stopReason; if (stopReason === "error") ...`) so the check stays type-safe without introducing `any`.
<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 the assistant message has an error stop reason, it might be filtered out | ||
| // by the provider on subsequent requests (e.g. Anthropic content filtering). | ||
| // We drop it to avoid orphaning any tool results that would be added below. | ||
| if ((assistant as any).stopReason === "error") { | ||
| changed = true; | ||
| continue; | ||
| } |
There was a problem hiding this comment.
[P2] Add a regression test for dropping stopReason === "error" assistant turns
This PR changes pairing behavior by skipping assistant messages with stopReason: "error", but src/agents/session-transcript-repair.test.ts doesn’t cover this case. A test with an errored assistant toolCall + subsequent toolResult would help ensure we keep dropping the assistant and don’t insert synthetic results for it (the #3860 regression).
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agents/session-transcript-repair.ts
Line: 120:126
Comment:
[P2] Add a regression test for dropping `stopReason === "error"` assistant turns
This PR changes pairing behavior by skipping assistant messages with `stopReason: "error"`, but `src/agents/session-transcript-repair.test.ts` doesn’t cover this case. A test with an errored assistant toolCall + subsequent toolResult would help ensure we keep dropping the assistant and don’t insert synthetic results for it (the #3860 regression).
How can I resolve this? If you propose a fix, please make it concise.bfc1ccb to
f92900f
Compare
|
Stale triage: superseded by merged #4598 (Feb 6, 2026). Main already includes stopReason error/aborted skip in session transcript repair. |
This PR fixes issue #3860 where assistant messages with stopReason 'error' (e.g., from content filtering) would have synthetic tool results added by the transcript repair logic. These tool results would then become orphaned if the provider filtered out the errored assistant message on subsequent requests, leading to 400 errors.
The fix ensures that assistant messages with an error stop reason are dropped during transcript repair, preventing the addition of synthetic tool results for them.
Greptile Overview
Greptile Summary
This PR updates
repairToolUseResultPairingto drop assistant messages whosestopReasonis"error", preventing transcript-repair from inserting synthetic tool results that could later become orphaned if a provider filters the errored assistant turn (the #3860 scenario). The change fits into the existing transcript sanitizer’s responsibilities: keeping tool calls/results strictly adjacent and removing free-floating toolResult entries so strict providers don’t reject requests.Main follow-ups: avoid the new
as anyaccess forstopReasonand add a regression test covering an errored assistant toolCall turn to ensure the fix stays in place.Confidence Score: 4/5
as any) and missing regression coverage for the new behavior, rather than functional correctness of the fix itself.Context used:
dashboard- CLAUDE.md (source)dashboard- AGENTS.md (source)