Repository navigation
Classify provider refusals once for every chat action - #1753
Merged
Merged
Conversation
Twitch refused an unban with HTTP 400 and the agent alerted with a 500: the refusal's wording mentions the "user_id query parameter", which the keyword check added in #1752 took for a malformed request. Twitch delete and clear, and every Kick action, had no refusal handling at all, so any explained 4xx from them alerted the same way. HTTPError now records whether the provider explained a rejection (a non-blank JSON message), never the text. The shared chat wrapper turns an explained 400, 404, 409 or 422 into a failed-precondition rejection for every action and provider, next to the existing 429 mapping. The wording heuristic is gone: the request shapes these actions send are pinned by the Twitch and Kick action tests instead, so agent bugs are caught before deploy rather than by guessing from provider text. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EzdqAWrbTMdtJutmK41coy
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.
Problem
Error group
CKSyptWhubTOeA(two events about 1s apart on0cf4831):This is a regression from #1752. That PR treated a 400 as a malformed request (and alerted) when Twitch's message mentioned
parameter|required|malformed|valid. Twitch's own unban refusal wording mentions the "user_id query parameter", so a normal "this user isn't banned" refusal was reported as an agent bug. I reproduced it with that wording againstmain: the response was a 500.The same problem showed up on other endpoints: Twitch
deleteMessage/clearand every Kick action (ban,unban,timeout,deleteMessage,send) had no refusal handling. Any 4xx where the provider explained its refusal turned into a 500 alert.Fix
Refusals are now classified in one place for all chat actions, and the provider's wording is no longer interpreted:
clients.HTTPError.Explainedrecords only whether the provider gave a reason (a non-blank JSONmessage, the field Twitch and Kick use). The text is never stored, so it can't leak into logs or responses.providerRejectionin the shared chat wrapper turns an explained 400/404/409/422 intofailed-precondition" rejected this request." The existing 429 →resource-exhaustedmapping moved into it. This applies to every action and every provider. YouTube keeps its own reason-based mapping.writeBan. Known Twitch phrasings ("already banned", "not banned", …) still get their specific messages.The "agent bugs should still alert" guarantee no longer depends on guessing from provider text at runtime. It now comes from test-time contracts:
TestTwitchModerationAndRaidsandTestKickActionsalready check the query parameters and body of every request these actions send.Tests
unban refusal naming its parameterrow, using Twitch-style wording. It fails onmainwith a 500 and passes here.TestProviderRejectionsAcrossEndpointscovers Twitch delete/clear and Kick ban/unban/delete across 400/404/409/422. It also checks that unexplained, non-JSON, non-string-message and 5xx responses still alert, and that provider text never reaches logs or responses.go test ./...passes except theinternal/youtubelive tests, which need youtube.com (blocked in the sandbox where this was developed).Known gaps
Generated by Claude Code