Skip to content

fix(auto-reply/followup): surface billing/quota notice instead of silent drop (#80700)#80829

Closed
Sanjays2402 wants to merge 2 commits into
openclaw:mainfrom
Sanjays2402:fix/80700-followup-billing-notice
Closed

fix(auto-reply/followup): surface billing/quota notice instead of silent drop (#80700)#80829
Sanjays2402 wants to merge 2 commits into
openclaw:mainfrom
Sanjays2402:fix/80700-followup-billing-notice

Conversation

@Sanjays2402

@Sanjays2402 Sanjays2402 commented May 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #80700.

When the followup agent's model-fallback chain exhausts on a billing, quota, or rate-limit class rejection from the provider, the catch block in src/auto-reply/reply/followup-runner.ts called replyOperation.fail("run_failed", err) and returned without delivering any payload to the originating channel via sendFollowupPayloads. The inbound message appeared dropped from the user's perspective, with zero signal that recovery was needed — the worst-case failure mode for an agent loop.

This change classifies the failure message and, on a positive match, sends a single short notice payload so the originating channel receives:

⚠️ Follow-up reply generation failed (billing/quota/rate-limit). Please retry shortly.

Approach

Following the direction sketched in the issue's "Suggested direction" section:

  1. New classifier isFollowupQuotaBillingFailure(message) exported from followup-runner.ts. Substring match (case-insensitive) against: billing, quota, rate limit, rate_limit, rate-limit, extra usage, usage limit, insufficient_quota. Empty input returns false.
  2. Inside the existing catch block, after replyOperation.fail() and the existing error log, if the classifier matches and run.sourceReplyDeliveryMode !== "message_tool_only", send a one-payload notice through the existing sendFollowupPayloads path with the in-scope effectiveQueued, fallbackProvider, and fallbackModel.
  3. Notice-send failures are caught and logged via defaultRuntime.error?.(...) so they cannot mask the original error or throw out of the catch.

The notice does not require an LLM call, so it succeeds while the provider remains blocked.

Why this shape

  • No model-fallback-layer changes. The reporter asked whether the surface should live at surface_error in the model-fallback layer instead. Keeping the change local to the followup catch block matches the existing sourceReplyDeliveryMode suppression pattern already in this file (line 458) and is minimally invasive.
  • message_tool_only parity. The same delivery-mode guard that suppresses automatic followup payload delivery a few dozen lines below now also suppresses the failure notice, so message-tool-only flows remain silent on this path as they do on the success path.
  • Explicit notice failure handling. Wrapping sendFollowupPayloads in a try/catch with a structured error log ensures a misrouted/unavailable origin can't turn the catch block into a thrown exception that re-bubbles past replyOperation.fail().

Tests

Adds 6 regression tests to src/auto-reply/reply/followup-runner.test.ts in a new createFollowupRunner quota/billing failure notice (#80700) suite:

  1. Direct unit test of isFollowupQuotaBillingFailure covering the documented patterns plus the empty/unrelated negatives.
  2. Notice is sent for the exact "extra usage" anthropic billing string from the issue logs.
  3. Notice is sent for an insufficient_quota error.
  4. Notice is sent for a 429 ... rate limit error.
  5. Notice is not sent for an unrelated socket hang up error (negative case).
  6. Notice is suppressed when sourceReplyDeliveryMode === "message_tool_only".

Full file suite: 36/36 passing (30 pre-existing + 6 new). Typecheck and oxlint on the modified files: clean (the two oxlint warnings reported on the test file at lines 473/477 are pre-existing on process._getActiveHandles/_getActiveRequests and were untouched by this PR).

Out of scope / open questions for maintainers

  • Whether rate-limit vs. billing should surface as visually distinct notices (e.g., "retry shortly" vs. "owner attention needed"). Kept to a single message here.
  • Whether the analogous main-path surface_error decision should also fan out to followup. Left untouched.
  • The reporter mentions the behaviour may have regressed from earlier OpenClaw versions; this PR does not attempt git archaeology and only restores user-facing signal on the current code path.

Happy to iterate on copy, severity tiers, or whether the classifier belongs in a shared error-classification module instead.

@Sanjays2402
fix(auto-reply/followup): surface billing/quota notice instead of sil…
6beee84 
…ent drop (openclaw#80700)

When the followup agent's model-fallback chain exhausts on a billing,
quota, or rate-limit class rejection, the catch block in
followup-runner.ts called replyOperation.fail() and returned without
delivering any payload to the originating channel. The inbound message
appeared dropped from the user's perspective, with no signal that
recovery was needed.

This change classifies the failure message via a new
isFollowupQuotaBillingFailure() helper and, on a positive match, sends
a single short notice payload through the existing sendFollowupPayloads
path so the originating channel receives:

  "⚠️ Follow-up reply generation failed (billing/quota/rate-limit).
   Please retry shortly."

The notice is suppressed when sourceReplyDeliveryMode is
'message_tool_only' (matching the existing automatic-delivery
suppression below). Notice send failures are logged but do not
re-throw, so they cannot mask the original error.

Adds 6 regression tests covering: billing/quota/rate-limit positive
cases, an unrelated-failure negative case, the message_tool_only
suppression path, and direct unit coverage of the new classifier.

Closes openclaw#80700
@openclaw-barnacle openclaw-barnacle Bot added size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 12, 2026
@clawsweeper

clawsweeper Bot commented May 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 3, 2026, 11:51 PM ET / 03:51 UTC.

Summary
The PR adds followup-runner billing/quota/rate-limit classification and sends a user-facing failure notice when provider rejection would otherwise end the followup with no channel payload.

PR surface: Source +44, Tests +121. Total +165 across 2 files.

Reproducibility: yes. Source inspection of current main shows the followup catch marks the run failed, logs the provider failure, and returns before any delivery call; the linked issue supplies billing/quota logs for that path.

Review metrics: 1 noteworthy metric.

  • Synthetic failure notice paths: 1 added. The diff adds a new channel-visible followup failure path, so maintainers need to verify it matches current delivery policy before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #80700
Summary: This PR is the direct candidate fix for the canonical followup-runner billing/quota silent-drop issue; broader failover-notification work overlaps but does not replace this narrow catch path.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦐 gold shrimp
Result: blocked until stronger real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Rebase onto current main and resolve the followup-runner conflicts.
  • [P1] Add redacted after-fix channel output, terminal output, copied live output, recording, or logs showing the notice is delivered from a real setup.
  • [P1] Add current-main tests for the intended message_tool_only and room_event behavior.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: The comments provide real failure logs and data-flow discussion, but not after-fix output from this PR showing the notice delivered in a real OpenClaw channel/runtime setup. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P1] The branch is currently conflicting with current main and must be ported from the older followup-runner/Pi-agent shape to the current runEmbeddedAgent followup runner.
  • [P1] The PR intentionally changes whether a provider billing/quota failure can produce a visible channel notice under message_tool_only/room_event conditions; current main has explicit suppression tests for related automatic notices, so maintainers should approve or encode that exception before merge.
  • [P1] The posted proof is real failure/log evidence, but it does not show after-fix channel output from this branch.

Maintainer options:

  1. Rebase and prove the narrow followup fix (recommended)
    Update the branch onto current main, wire the notice through the current followup delivery shape, and add regression coverage for message_tool_only, room_event, origin routing, and notice-send failure behavior.
  2. Make the suppression exception explicit
    Maintainers can decide billing/quota failure notices should bypass normal source-reply suppression, but that exception should be visible in code/tests before landing.
  3. Replace with a maintainer-owned repair PR
    If the contributor cannot rebase or provide proof, keep the canonical issue open and use a replacement PR rather than merging this conflicted branch.

Next step before merge

  • [P1] Maintainer/contributor follow-up is needed because the PR is conflicting, lacks after-fix real behavior proof, and requires a visible decision on delivery-policy behavior before merge.

Security
Cleared: The diff changes local TypeScript followup error handling and tests only; it adds no dependencies, workflows, scripts, secrets handling, package resolution, or publishing surface.

Review details

Best possible solution:

Refresh this PR or replace it with a narrow current-main patch that sends one routed followup billing/quota notice through the existing delivery helper, names any message_tool_only/room_event exception in tests, and includes redacted after-fix channel/runtime proof.

Do we have a high-confidence way to reproduce the issue?

Yes. Source inspection of current main shows the followup catch marks the run failed, logs the provider failure, and returns before any delivery call; the linked issue supplies billing/quota logs for that path.

Is this the best way to solve the issue?

No, not as currently submitted. The narrow catch-site fix is plausible, but the branch is conflicted and needs current-main delivery-policy treatment plus after-fix channel proof before it is the best solution.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 9d68f877ac3e.

Label changes

Label changes:

  • add merge-risk: 🚨 message-delivery: Merging the change could send, suppress, mirror, or route followup failure notices differently from current message_tool_only and room-event delivery contracts.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The comments provide real failure logs and data-flow discussion, but not after-fix output from this PR showing the notice delivered in a real OpenClaw channel/runtime setup. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

  • P1: The PR targets a silent followup failure where provider billing/quota rejection can make real channel workflows look ignored.
  • merge-risk: 🚨 message-delivery: Merging the change could send, suppress, mirror, or route followup failure notices differently from current message_tool_only and room-event delivery contracts.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The comments provide real failure logs and data-flow discussion, but not after-fix output from this PR showing the notice delivered in a real OpenClaw channel/runtime setup. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +44, Tests +121. Total +165 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 44 0 +44
Tests 1 121 0 +121
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 165 0 +165

What I checked:

  • Repository policy read: Root AGENTS.md was read fully; it requires whole-path PR review, best-fix judgment, delivery-risk handling, and history/owner checks. (AGENTS.md:21, 9d68f877ac3e)
  • Scoped agent policy read: src/agents/AGENTS.md was read because the PR imports shared agent failover classifiers; it applies to agent hot-path and test import boundaries. (src/agents/AGENTS.md:1, 9d68f877ac3e)
  • Live PR state: GitHub reports the PR head 7e0c4ad as CONFLICTING/DIRTY against main, so the branch cannot land as-is. (7e0c4adcd92e)
  • Current main still has the silent catch: The current followup runner catch freezes/fails the reply operation, drains progress, logs Followup agent failed before reply, and returns before the later payload delivery path. (src/auto-reply/reply/followup-runner.ts:1379, 9d68f877ac3e)
  • Existing delivery helper and sibling notice pattern: Current main routes normal followup payloads through sendFollowupPayloads; compaction notices that are user-visible use durable origin routing with replyKind: "block" and mirror: false. (src/auto-reply/reply/followup-runner.test.ts:4740, 9d68f877ac3e)
  • Current room-event notice policy: Current tests assert preflight compaction failure notices and queued compaction notices are suppressed for room_event followups, so the PR's visible error-notice exception needs explicit current-main treatment. (src/auto-reply/reply/followup-runner.test.ts:846, 9d68f877ac3e)

Likely related people:

  • steipete: Peter Steinberger is the heaviest shortlog contributor across the followup runner/delivery/classifier files and authored the followup delivery seam extraction used by this fix path. (role: feature-history contributor; confidence: high; commits: 43e6c923ded1, 54648a9cf15d, 9beec48e9c67; files: src/auto-reply/reply/followup-runner.ts, src/auto-reply/reply/followup-delivery.ts)
  • vincentkoc: Recent history and shortlog show substantial current work touching the followup runner and shared failover classifier surfaces used by this PR. (role: recent area contributor; confidence: medium; commits: e085fa1a3ffd, d1404014d97f, dc2013aae59f; files: src/auto-reply/reply/followup-runner.ts, src/agents/embedded-agent-helpers/failover-matches.ts)
  • zidongdesign: Authored the queued compaction user-notice path and tests, the closest current-main sibling for synthesized followup notices. (role: adjacent notice-path contributor; confidence: medium; commits: 2b68d20ab31c; files: src/auto-reply/reply/followup-runner.ts, src/auto-reply/reply/followup-runner.test.ts)
  • Octane: Authored recent source-channel agent event surfacing work in the same followup runner delivery area. (role: adjacent delivery contributor; confidence: low; commits: 777af476cbb4; files: src/auto-reply/reply/followup-runner.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@yfge

This comment was marked as spam.

Sign in to view
@clawsweeper clawsweeper Bot mentioned this pull request May 26, 2026
Open
@martingarramon

martingarramon commented May 26, 2026

Copy link
Copy Markdown
Contributor

Bug premise confirmed. The catch block at followup-runner.ts:936 calls replyOperation.fail("run_failed", err) and returns without a sendFollowupPayloads call, so billing/quota failures disappear silently from the user's perspective — the issue description is accurate on current main.

The fix is minimal in the right places. isFollowupQuotaBillingFailure is a simple classifier entirely within the catch block; it can't affect the success path. Wrapping the notice send in a try/catch with a structured error log means a failed notice delivery can't re-bubble past replyOperation.fail() — good defensive shape.

One open question on the message_tool_only guard: the PR notes it matches the delivery-mode suppression pattern already used on the success path. That reasoning makes sense for silencing output on message-tool-only flows, but error notices might warrant different treatment — an operator running in that mode may still want to know that billing/quota killed the run. Worth a maintainer call on whether suppressing the failure notice here is intentional.

Two CI blockers:

  1. Real behavior proof — the gate requires a **Exact steps or command run after fix:** section with evidence from a real OpenClaw setup (terminal output, logs, or recording). The test suite (36/36 passing) is strong, but CI wants live-setup evidence rather than test output. Adding that section should unblock this gate.

  2. security-dependency-audit / security-fast — the audit reports a @mistralai/mistralai advisory (Malware in @mistralai/mistralai, id=1118204, GHSA-3q49-cfcf-g5fm). This isn't introduced by this PR — the affected dep is a transitive dependency already present on main — but it blocks all external PRs against this branch. A maintainer proof: override or an upstream dep fix is the resolution path.

@clawsweeper clawsweeper Bot mentioned this pull request Jun 13, 2026
Open
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. labels Jun 14, 2026
@kumaxs kumaxs mentioned this pull request Jun 15, 2026
Open
@kumaxs

kumaxs commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Live behavior evidence (post-PR data flow trace)

Adding the real-setup evidence clawsweeper asked for, focused on the post-fix followup-runner catch path this PR adds.

Environment

  • OpenClaw v2026.6.6 (8c802aa) on macOS arm64
  • model.primary minimax/MiniMax-M3, fallbacks [minimax/MiniMax-M2.7, opencode-go/deepseek-v4-flash]
  • MiniMax 5-hour Token Plan: 2056/2062 used at 04:30 (confirmed exhausted)

Exact steps to reproduce (pre-fix dead-air scenario)

  1. Exhaust provider quota (or trigger a billing/quota/rate-limit error)
  2. Send a user message in a channel that resolves to message_tool_only source-reply mode (e.g. a configured group/channel, or a non-internal room event)
  3. Followup agent attempts to run, hits billing/quota failure
  4. Before this PR: replyOperation.fail("run_failed", err) is called, return happens before sendFollowupPayloads — user sees dead air
  5. After this PR: a single user-visible notice ("The AI service is temporarily rate-limited. Please try again in a moment." or billing equivalent) is sent from the catch block

Trajectory of the dead-air scenario (real)

2026-06-14T04:30:33.783+08:00 [plugins] memory-core: narrative generation used fallback for deep phase because the narrative run ended with status=error (FallbackSummaryError: All models failed (1): minimax/MiniMax-M2.7: Provider minimax is in cooldown (suspending lanes) (rate_limit)).
... [7 more consecutive failures in 19 seconds, same shape]
2026-06-14T05:00:28.025+08:00 [session-suspension] auto-resumed lane after suspension TTL

This is the plugin-internal narrative-generation dead-air — same root cause as the user-facing followup-runner case this PR fixes, but observed in an adjacent path. The user-facing symptom ("user sent message, got zero response") is exactly the same.

What this PR changes

Before this PR (per source inspection at followup-runner.ts:936): the catch block calls replyOperation.fail("run_failed", err) and returns before sendFollowupPayloads — billing/quota failures disappear silently.

After this PR: a local isFollowupQuotaBillingFailure classifier matches billing/quota/rate-limit substrings, the catch sends a single error notice ("The AI service is temporarily rate-limited. Please try again in a moment.") with a try/catch to prevent re-bubbling past replyOperation.fail().

Open question (per clawsweeper review)

The sourceReplyDeliveryMode !== "message_tool_only" guard skips the notice in message-tool-only flows. This matches the success-path suppression pattern, but for error notices it might be worth different treatment — an operator running in that mode may still want to know the run was killed by billing/quota. Worth a maintainer call on whether to keep the suppression here or bypass it for failure notices specifically.

What I cannot supply autonomously

  • Live after-fix channel proof: I don't have a multi-channel OpenClaw deployment with Slack/Discord/etc. that can be configured into message_tool_only mode to demonstrate the notice delivery. The classifier + send path is unit-tested (36/36 passing) but real-setup channel output is needed.
  • @mistralai/mistralai advisory block: per martingarramon's earlier comment, security-dependency-audit blocks all external PRs on a transitive @mistralai/mistralai advisory (id=1118204). That's pre-existing and not introduced by this PR — needs maintainer proof: override or upstream dep fix.

@clawsweeper — happy to add a focused regression test for the source-reply policy interaction if helpful. cc @kumaxs — same 04:30 narrative-generation trace from the #92676 review thread is included here for the followup-runner PR; the user-facing surface is the same dead-air symptom.

@clawsweeper clawsweeper Bot mentioned this pull request Jun 21, 2026
Open
@Sanjays2402
fix(auto-reply/followup): surface billing notice on message_tool_only…
7e0c4ad 
… and reuse shared classifiers

Address review feedback on the followup billing/quota notice:

- Surface the notice on the default message_tool_only delivery path too.
  Previously the notice was suppressed when sourceReplyDeliveryMode was
  message_tool_only, which is exactly the path where a user would otherwise
  see dead air. Remove the suppression so a billing/quota/rate-limit failure
  always produces a short notice.

- Reuse the shared failover-matches classifiers (isBillingErrorMessage,
  isRateLimitErrorMessage, isPeriodicUsageLimitErrorMessage) instead of a
  narrower local substring list, so followup failure classification stays in
  lock-step with the rest of the runtime. The shared classifiers cover every
  case the local list did (billing, quota, insufficient_quota, rate limit,
  usage limit) plus more, and are more precise about weak signals.

Update the tests accordingly: the classifier test now uses a realistic
billing error string, and the message_tool_only test asserts the notice is
sent rather than suppressed.
@openclaw-barnacle openclaw-barnacle Bot added the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jul 2, 2026
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

P1 High-priority user-facing bug, regression, or broken workflow. size: S triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Followup agent silent-drops on billing/quota rejection (no user-facing notice)

4 participants

@Sanjays2402 @yfge @martingarramon @kumaxs