fix: prevent false positive billing error detection in sanitizeUserFacingText#12777
fix: prevent false positive billing error detection in sanitizeUserFacingText#12777jpaine wants to merge 1 commit intoopenclaw:mainfrom
Conversation
|
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. |
|
Great approach — gating Relationship to #12988#12988 (merged to This PR is still needed because What needs updatingThe PR needs a rebase onto current 1. The diff targets stale code. The hunk header shows export function sanitizeUserFacingText(text: string, opts?: { errorContext?: boolean }): string {The billing check moved from line ~424 (unconditional) to line 526 (inside 2. Tests need On current Tests should use inputs that actually trigger // False positive — tool/upstream 402, not an LLM billing error.
// Should pass through even in error context.
expect(sanitizeUserFacingText("Got a 402 from the upstream API", { errorContext: true }))
.not.toContain("billing");
// Real billing error payload — should still rewrite
expect(sanitizeUserFacingText("402 Payment Required", { errorContext: true }))
.toContain("billing");
// JSON billing error — should still rewrite
expect(
sanitizeUserFacingText(
'{"type":"error","error":{"message":"insufficient credits","type":"billing_error"}}',
{ errorContext: true },
),
).toContain("billing"); |
jpaine
force-pushed
the
fix/sanitize-billing-false-positive
branch
from
fb52c02 to
79e3ea2
Compare
thewilloftheshadow
force-pushed
the
main
branch
from
bfc1ccb to
f92900f
Compare
|
Closing as superseded by the merged sanitize/error-context work:
This PR’s intent appears covered by those merged changes and current mainline tests. |
3 participants
Summary
sanitizeUserFacingText()pattern-matches/\b402\b/viaisBillingErrorMessage()on all assistant text before channel delivery. This causes any response containing the literal number "402" in normal context (e.g., "$402.55" in a cost report, street addresses, item counts) to be silently replaced with a billing error warning on the channel surface.Fixes #12711.
lobster-biscuit
Repro Steps
⚠️ API provider returned a billing error...Root Cause
sanitizeUserFacingText()callsisBillingErrorMessage(trimmed)unconditionally on all user-facing text. The billing error patterns include/\b402\b/, which matches "402" at any word boundary — including after$(since$is not a word character). Normal dollar amounts like "$402.55", addresses like "402 Main Street", and counts like "402 items" all trigger the false positive.The context overflow check in the same function already gates itself behind error-like indicators (
isRawApiErrorPayload,isLikelyHttpErrorText,ERROR_PREFIX_RE), but the billing check had no such guard.Behavior Changes
sanitizeUserFacingText()now only rewrites billing errors when the text also looks like a leaked error payload (raw API JSON, HTTP status line, or error-prefixed text)isBillingErrorMessage()itself is unchanged — it remains sensitive for use informatAssistantErrorText()andclassifyFailoverReason()where the input is already known to be an error message"payment"toHTTP_ERROR_HINTSso "402 Payment Required" is properly recognized as an HTTP errorCodebase and GitHub Search
sanitizeUserFacingText(normalize-reply, agent-runner-execution, sessions-helpers, pi-embedded-utils)isBillingErrorMessageto confirm the function itself should not change (used in formatAssistantErrorText and classifyFailoverReason where input is known-error)shouldRewriteContextOverflowTextwhich already gates context overflow detection behind error indicatorsTests
All pass (
pnpm check+pnpm test):$402.55), street addresses, item counts, historical yearsisBillingErrorMessagetests unchanged and passingisCompactionFailureErrortest is unrelated (present on main)Sign-Off
Made with Cursor
Greptile Overview
Greptile Summary
This PR adjusts
sanitizeUserFacingText()to avoid rewriting normal assistant output into the billing warning when the text merely contains the number402(e.g.$402.55). It does this by gating the billing rewrite behind additional “looks like an error payload” checks (isRawApiErrorPayload,isLikelyHttpErrorText, orERROR_PREFIX_RE). It also adds"payment"toHTTP_ERROR_HINTSso plain HTTP status lines like402 Payment Requiredare recognized as HTTP-ish error text, and adds tests covering both the non-rewrite and rewrite cases.Confidence Score: 5/5
sanitizeUserFacingText(), and the new gating logic directly addresses the reported false positive without changingisBillingErrorMessage()behavior for known-error contexts. Added tests cover the new behavior, and theHTTP_ERROR_HINTStweak aligns with existingisLikelyHttpErrorText()semantics. No additional call sites or invariants appear to be broken by this change.(2/5) Greptile learns from your feedback when you react with thumbs up/down!