fix: prevent sanitizeUserFacingText false-positives on 402 in normal text#12720
Closed
zerone0x wants to merge 1 commit intoopenclaw:mainfrom
Closed
fix: prevent sanitizeUserFacingText false-positives on 402 in normal text#12720zerone0x wants to merge 1 commit intoopenclaw:mainfrom
zerone0x wants to merge 1 commit intoopenclaw:mainfrom
Conversation
…nt prose `sanitizeUserFacingText()` applied error-detection heuristics (including `isBillingErrorMessage` with `/\b402\b/`) to all outbound text, causing normal assistant responses containing "402" (e.g. "$402.55 MTD spend") to be replaced with a billing error warning on messaging channels. Two-pronged fix: 1. Add `looksLikeAssistantProse()` guard that detects multi-sentence text, paragraph breaks, and markdown formatting — structural signals that distinguish assistant prose from short API error messages. Skip all heuristic error checks when text looks like prose. 2. Tighten the `/\b402\b/` regex in ERROR_PATTERNS.billing to require HTTP error context (e.g. "HTTP 402 Payment Required", "error 402") instead of matching bare "402" in any word-boundary position. JSON error payloads (`isRawApiErrorPayload`) are still caught regardless of text length since they're structurally identifiable. Fixes openclaw#12711 Co-Authored-By: Claude <[email protected]>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+583
to
+589
| // Skip both strict and loose heuristics for text that looks like normal | ||
| // assistant prose — real API billing errors are always short, single-line | ||
| // messages, not multi-sentence responses. | ||
| // Note: pass original `raw` to preserve case for sentence boundary detection. | ||
| if (looksLikeAssistantProse(raw)) { | ||
| return false; | ||
| } |
Contributor
There was a problem hiding this comment.
Billing errors skipped
isBillingErrorMessage() bails out when looksLikeAssistantProse(raw) is true (errors.ts:583-589). Since looksLikeAssistantProse() returns true for any \n\n or markdown constructs, a real provider billing error that includes a blank line or formatted/wrapped text will be missed and won’t get classified as billing (affects formatAssistantErrorText, sanitizeUserFacingText, and classifyFailoverReason). Consider limiting this guard to the loose keyword branch only, or narrowing the prose checks so they don’t exclude structured/HTTP billing errors.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agents/pi-embedded-helpers/errors.ts
Line: 583:589
Comment:
**Billing errors skipped**
`isBillingErrorMessage()` bails out when `looksLikeAssistantProse(raw)` is true (`errors.ts:583-589`). Since `looksLikeAssistantProse()` returns true for any `\n\n` or markdown constructs, a real provider billing error that includes a blank line or formatted/wrapped text will be missed and won’t get classified as billing (affects `formatAssistantErrorText`, `sanitizeUserFacingText`, and `classifyFailoverReason`). Consider limiting this guard to the *loose keyword* branch only, or narrowing the prose checks so they don’t exclude structured/HTTP billing errors.
How can I resolve this? If you propose a fix, please make it concise.
Contributor
|
Fixed in #12988. This will go out in the next OpenClaw release. If you still see this after updating to the first release that includes #12988, please open a new issue with:
Link back here for context. |
thewilloftheshadow
force-pushed
the
main
branch
from
bfc1ccb to
f92900f
Compare
Contributor
|
Closing as superseded by the merged sanitize/error-context work:
This PR’s intent appears covered by those merged changes and current mainline tests. |
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.
Summary
Fixes #12711
sanitizeUserFacingText()callsisBillingErrorMessage()on all outbound messaging text. The/\b402\b/regex matches "402" in any word-boundary context — including$402.55in cost reports — replacing the entire response with the billing error warning on channel surfaces (Signal, Telegram, etc.).Root Cause
Two issues combine to create the false positive:
/\b402\b/matches "402" in$402.55because$is not a word character (word boundary at$→4)sanitizeUserFacingTextapplies error heuristics to all text equally, whether it's a short API error or a multi-paragraph assistant responseChanges
looksLikeAssistantProse()guard — detects multi-sentence text (3+ sentences), paragraph breaks (\n\n), and markdown formatting. When text looks like prose, skip all heuristic error checks (billing, HTTP, error prefix)/\b402\b/with context-requiring patterns (/(?:HTTP[/ ]\s*|status[: ]\s*|^)402\s+payment\s+required/iand/\berror\b.*\b402\b/i). The existing"payment required"string pattern already catches the real error caseisRawApiErrorPayloadis structurally identifiable (JSON structure) and should always be caught regardless of text lengthisBillingErrorMessage— the loose keyword branch (billing+upgrade/credits/payment/plan) also skips proseTest Plan
pi-embedded-helperstests pass$402.55in dollar amountspnpm lint(0 errors),pnpm build,pnpm formatall pass🤖 Generated with Claude Code (issue-hunter-pro)
Greptile Overview
Greptile Summary
This PR reduces false-positive sanitization of normal assistant responses by adding a
looksLikeAssistantProse()guard insanitizeUserFacingText(), tightening the 402 billing regex to require HTTP/error context, and prioritizing structurally-identifiable JSON error payload detection. It also adds test coverage for previously problematic cases like$402.55in cost reports and multi-paragraph/markdown assistant text.Main integration point is
src/agents/pi-embedded-helpers/errors.ts, which is used by multiple helpers (sanitizeUserFacingText,formatAssistantErrorText, and failover classification) to decide when to rewrite/label outbound text as an error versus leaving it as user-facing content.Confidence Score: 4/5
isBillingErrorMessage()that can suppress legitimate billing detection for errors containing blank lines/markdown wrappers, which can impact user messaging and failover classification.