fix: prevent 402 regex from matching currency amounts in responses#12726
fix: prevent 402 regex from matching currency amounts in responses#12726brandonwise wants to merge 2 commits intoopenclaw:mainfrom
Conversation
The billing error detector used \b402\b which matched legitimate numbers like $402.55. Now only matches in error response context. Changes: - Added looksLikeErrorText() guard function to detect actual error messages - Updated sanitizeUserFacingText() to only apply billing error detection when text looks like an error (API payloads, HTTP errors, error prefixes) - Added 'payment required' to HTTP_ERROR_HINTS for HTTP 402 detection - Added tests for currency amounts containing 402 (issue reproduction) - Added tests verifying actual 402 billing errors are still caught Fixes openclaw#12711
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // Only apply billing error detection when text looks like an error message, | ||
| // not on normal assistant output that might contain numbers like "$402.55" | ||
| if (looksLikeErrorText(trimmed) && isBillingErrorMessage(trimmed)) { | ||
| return BILLING_ERROR_USER_MESSAGE; | ||
| } |
There was a problem hiding this comment.
Plain billing errors bypassed
sanitizeUserFacingText() now only rewrites billing errors when looksLikeErrorText(trimmed) is true. But looksLikeErrorText() doesn’t match common plain-text billing messages that isBillingErrorMessage() is explicitly designed to catch (e.g. "Payment Required", "insufficient credits", "credit balance", "plans & billing"). Those will now fall through and be returned verbatim instead of BILLING_ERROR_USER_MESSAGE, which is a behavior regression on the user-facing sanitization path (used in channel delivery/streaming).
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agents/pi-embedded-helpers/errors.ts
Line: 446:450
Comment:
**Plain billing errors bypassed**
`sanitizeUserFacingText()` now only rewrites billing errors when `looksLikeErrorText(trimmed)` is true. But `looksLikeErrorText()` doesn’t match common plain-text billing messages that `isBillingErrorMessage()` is explicitly designed to catch (e.g. `"Payment Required"`, `"insufficient credits"`, `"credit balance"`, `"plans & billing"`). Those will now fall through and be returned verbatim instead of `BILLING_ERROR_USER_MESSAGE`, which is a behavior regression on the user-facing sanitization path (used in channel delivery/streaming).
How can I resolve this? If you propose a fix, please make it concise.Address greptile review feedback: plain billing messages like 'insufficient credits' or 'credit balance' would bypass the new looksLikeErrorText() guard since they don't look like HTTP or API error payloads. Add hasExplicitBillingKeywords() helper to detect unambiguous billing keywords that are safe to match (unlike bare '402'). The billing check now fires when EITHER: - Text looks like an error message (looksLikeErrorText), OR - Text contains explicit billing keywords This preserves the false-positive fix while ensuring real billing errors are still caught regardless of their format. Add test coverage for plain-text billing scenarios.
|
Thanks for catching this! You're right that plain-text billing messages would slip through. Fixed in the latest push — added The billing check now fires when EITHER:
This preserves the $402.55 false-positive fix while ensuring real billing errors are caught regardless of format. Added test coverage for the plain-text scenarios too. |
|
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. |
|
Closing in favor of #12988 which takes a more comprehensive approach by scoping all sanitization rewrites to require explicit error context. Thanks for the fix, @Takhoffman! 🙌 |
2 participants
Fixes #12711
What
Fixes false positive billing error detection that replaced normal responses containing '402' with error warnings.
Why
The
isBillingErrorMessage()regex/\b402\b/matches any word-bounded '402', including currency amounts like '$402.55'. This caused legitimate assistant responses to be replaced with billing error messages on channel delivery.How
Added a
looksLikeErrorText()guard function that checks if text appears to be an actual error message (JSON API payloads, HTTP status codes with error hints, or text starting with error prefixes). The billing error check insanitizeUserFacingText()now only applies when this guard returns true.Also added 'payment required' to
HTTP_ERROR_HINTSso that "402 Payment Required" HTTP errors are still properly detected.Testing
pnpm checkpassespnpm testpasses (related tests)AI-Assisted
This PR was built with AI assistance (Claude via OpenClaw). The code has been reviewed, tested, and I understand what it does.
Greptile Overview
Greptile Summary
This PR adjusts billing-error sanitization to avoid false positives when the assistant output contains the substring
402in normal text (e.g. currency amounts). It introduces alooksLikeErrorText()guard and expands HTTP error hints to include "payment required", plus adds unit tests covering currency/room-number cases and ensuring HTTP 402 / JSON billing payloads are still caught.The change integrates into the existing error-rewrite pipeline by gating the
isBillingErrorMessage()rewrite insidesanitizeUserFacingText(), which is used by reply normalization/channel delivery.Confidence Score: 3/5
looksLikeErrorText()gate appears to prevent rewriting of several real billing errors that arrive as plain text (not HTTP-status-line or JSON), which can change user-visible behavior in channel delivery.(4/5) You can add custom instructions or style guidelines for the agent here!