fix(agents): detect empty provider responses as failures, improve Telegram error routing by garnetlyx · Pull Request #60830 · openclaw/openclaw

garnetlyx · 2026-04-04T11:10:47Z

Summary

Adds EmptyResponseError typed class in model-fallback.ts; replaces the fragile string-matched new Error("Empty response content received from provider")
In runFallbackAttempt: treats a raw provider response with content: [] and no meta field as a failure, triggering fallback to the next candidate (benefits all channels)
In Telegram dispatch: tracks lastError via onError and an outer try/catch; routes the user-visible fallback message by error type (rate limit / empty response / general); re-throws aborts and timeouts so they don't produce spurious fallback replies; re-throws other dispatcher crashes after cleanup so callers still see the failure for logging/alerting
Adds tests for the empty-content detection (triggers fallback) and the meta field guard (accepted as valid)

fix(channels): extend user-visible error fallback to Discord, Slack, Signal, WhatsApp #60812 — extend user-visible error fallback to Discord, Slack, Signal, WhatsApp
fix(line): upgrade hardcoded error fallback to typed error routing #60813 — upgrade LINE's hardcoded error fallback to typed error routing

Test plan

pnpm test src/agents/model-fallback.test.ts — all tests pass including 3 new ones
bunx oxlint src/agents/model-fallback.ts src/telegram/bot-message-dispatch.ts — 0 errors
Telegram: provider returns empty content → user sees "I wasn't able to generate a response. Please try rephrasing your question."
Telegram: rate-limited provider → user sees rate limit message
Telegram: /stop or timeout → no spurious fallback reply sent

…egram error routing

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fb3b7e10a6

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-04-04T11:16:03Z

+  // Re-throw dispatcher crashes after fallback/cleanup so callers can log/alert
+  if (caughtDispatchError) {
+    throw caughtDispatchError;


Re-throw dispatch crashes before early no-response return

When dispatchReplyWithBufferedBlockDispatcher throws and caughtDispatchError is set, the function can still exit via the !hasFinalResponse early return (for example if fallback delivery also fails or only partial/non-final output was sent), so the new re-throw block is never reached. This silently swallows dispatcher crashes that previously propagated, which can hide production failures from upstream logging/alerting and make incident triage harder.

Useful? React with 👍 / 👎.

greptile-apps · 2026-04-04T11:16:25Z

Greptile Summary

This PR introduces two improvements: a typed EmptyResponseError class replacing string-matched error detection in the model fallback pipeline, and improved Telegram error routing that sends typed user-visible fallback messages (rephrasing prompt for empty responses, rate-limit message, or generic error) while correctly suppressing spurious fallback replies on abort/timeout.

Key changes:

EmptyResponseError class (model-fallback.ts): Clean replacement for new Error(\"Empty response content received from provider\"); exported so downstream consumers (Telegram dispatch) can use instanceof checks for typed routing.
runFallbackAttempt empty-content guard: A runtime check treats { content: [], ...no meta } results as failures, triggering fallback to the next candidate. The meta field is used as a heuristic to distinguish raw provider responses from EmbeddedPiRunResult wrappers — functional but not type-enforced.
Telegram dispatch error tracking (bot-message-dispatch.ts): Adds lastError/caughtDispatchError tracking. Abort and timeout errors are re-thrown early (before the finally block) to prevent spurious fallback replies on /stop or timeout. Other dispatch failures result in a typed fallback message, then the original error is re-thrown for callers to log/alert.
Tests: Three new unit tests cover the primary scenarios. One edge case (single candidate returns empty content → EmptyResponseError thrown directly) is not tested.

Confidence Score: 4/5

Safe to merge with minor observability and test-coverage gaps; no runtime correctness issues found.

The core logic is sound: EmptyResponseError propagates correctly through the fallback loop and into the Telegram dispatch routing in all scenarios. Abort/timeout re-throw prevents spurious fallback replies. The only concerns are: (1) the meta-field heuristic in runFallbackAttempt is not type-enforced and could silently break if either EmbeddedPiRunResult or raw provider response shapes change; (2) EmptyResponseError from the last/only candidate bypasses attempts.push and params.onError, reducing observability; (3) the single-candidate empty-content path is not covered by tests. None of these are correctness bugs in the current code.

src/agents/model-fallback.ts (meta-field guard and last-attempt observability); src/agents/model-fallback.test.ts (single-candidate empty-content test coverage)

Comments Outside Diff (1)

src/agents/model-fallback.ts, line 550-552 (link)

Last-attempt EmptyResponseError skips attempts push and onError callback

When EmptyResponseError is the error returned from the last (or only) candidate, isKnownFailover is false, so throw err is hit before attempts.push(...) and params.onError?.() are reached. This means:

The final attempt is absent from the attempts array, so throwFallbackFailureSummary is also bypassed — callers only receive the raw EmptyResponseError, not the structured "All models failed" summary that multi-attempt chains normally produce.
Any monitoring wired via params.onError never sees the terminal failure.

This is the same behavior as for any other unrecognized error at the last position, but EmptyResponseError is now a deliberately generated, typed error (not an accidental exception), so missing it in the onError callback is more surprising. If observability for this case matters, consider recording the attempt before re-throwing, or registering EmptyResponseError as a known failover reason.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/agents/model-fallback.ts
Line: 550-552

Comment:
**Last-attempt `EmptyResponseError` skips `attempts` push and `onError` callback**

When `EmptyResponseError` is the error returned from the last (or only) candidate, `isKnownFailover` is `false`, so `throw err` is hit before `attempts.push(...)` and `params.onError?.()` are reached. This means:

- The final attempt is absent from the `attempts` array, so `throwFallbackFailureSummary` is also bypassed — callers only receive the raw `EmptyResponseError`, not the structured "All models failed" summary that multi-attempt chains normally produce.
- Any monitoring wired via `params.onError` never sees the terminal failure.

This is the same behavior as for any other unrecognized error at the last position, but `EmptyResponseError` is now a deliberately generated, typed error (not an accidental exception), so missing it in the `onError` callback is more surprising. If observability for this case matters, consider recording the attempt before re-throwing, or registering `EmptyResponseError` as a known failover reason.

How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: src/agents/model-fallback.ts
Line: 163-175

Comment:
**Fragile `meta`-field discriminant for EmbeddedPiRunResult**

The `!("meta" in result)` guard is load-bearing for correctness: if a raw provider ever adds a `meta` field to its empty-content response (or if `EmbeddedPiRunResult` ever drops its `meta` field), the detection will silently misfire in the wrong direction. Since this heuristic is also completely invisible in the type system, future callers of `runWithModelFallback` have no way to know their result type needs to satisfy the guard.

A more robust approach would be to either:
1. Pass an explicit `isRawProviderResult` type predicate into `runFallbackAttempt`, or
2. Require callers to mark which result types should trigger the empty-content check (e.g. a wrapper type or a config flag)

The current approach is an improvement over the prior string-matched check, but it introduces a new latent coupling between `runFallbackAttempt`'s heuristic and the shape of `EmbeddedPiRunResult` that isn't enforced anywhere in the type system.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/agents/model-fallback.ts
Line: 550-552

Comment:
**Last-attempt `EmptyResponseError` skips `attempts` push and `onError` callback**

When `EmptyResponseError` is the error returned from the last (or only) candidate, `isKnownFailover` is `false`, so `throw err` is hit before `attempts.push(...)` and `params.onError?.()` are reached. This means:

- The final attempt is absent from the `attempts` array, so `throwFallbackFailureSummary` is also bypassed — callers only receive the raw `EmptyResponseError`, not the structured "All models failed" summary that multi-attempt chains normally produce.
- Any monitoring wired via `params.onError` never sees the terminal failure.

This is the same behavior as for any other unrecognized error at the last position, but `EmptyResponseError` is now a deliberately generated, typed error (not an accidental exception), so missing it in the `onError` callback is more surprising. If observability for this case matters, consider recording the attempt before re-throwing, or registering `EmptyResponseError` as a known failover reason.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/agents/model-fallback.test.ts
Line: 1210-1230

Comment:
**Missing test for single-candidate empty-content scenario**

The new test only covers the two-candidate case (primary returns `{ content: [] }`, fallback returns `"ok"`). The single-candidate path is meaningfully different: `isKnownFailover` is false and `i === candidates.length - 1`, so `EmptyResponseError` is thrown directly (before `attempts.push` and `onError`), bypassing `throwFallbackFailureSummary`.

It would be good to add a test asserting that when the only candidate returns `{ content: [] }`, the call rejects with an `EmptyResponseError` (not a generic "All models failed" wrapper):

```ts
it("throws EmptyResponseError when the only candidate returns empty content", async () => {
  const cfg = makeCfg({
    agents: { defaults: { model: { primary: "openai/gpt-4.1-mini" } } },
  });
  const run = vi.fn().mockResolvedValueOnce({ content: [] });
  await expect(
    runWithModelFallback({ cfg, provider: "openai", model: "gpt-4.1-mini", run }),
  ).rejects.toBeInstanceOf(EmptyResponseError);
  expect(run).toHaveBeenCalledTimes(1);
});
```

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix(agents): detect empty provider respo..." | Re-trigger Greptile}

greptile-apps · 2026-04-04T11:16:28Z

+    // Check for empty raw provider response content array (treat as failure)
+    // Do NOT check EmbeddedPiRunResult payloads here - that should be handled upstream
+    // to avoid breaking valid use cases like memory flush which intentionally return empty
+    const result = runResult.result;
+    if (
+      result &&
+      typeof result === "object" &&
+      "content" in result &&
+      Array.isArray(result.content) &&
+      result.content.length === 0 &&
+      // Only apply to raw provider responses (not wrapped in EmbeddedPiRunResult)
+      !("meta" in result)
+    ) {


Fragile meta-field discriminant for EmbeddedPiRunResult

The !("meta" in result) guard is load-bearing for correctness: if a raw provider ever adds a meta field to its empty-content response (or if EmbeddedPiRunResult ever drops its meta field), the detection will silently misfire in the wrong direction. Since this heuristic is also completely invisible in the type system, future callers of runWithModelFallback have no way to know their result type needs to satisfy the guard.

A more robust approach would be to either:

Pass an explicit isRawProviderResult type predicate into runFallbackAttempt, or

Require callers to mark which result types should trigger the empty-content check (e.g. a wrapper type or a config flag)

The current approach is an improvement over the prior string-matched check, but it introduces a new latent coupling between runFallbackAttempt's heuristic and the shape of EmbeddedPiRunResult that isn't enforced anywhere in the type system.

Prompt To Fix With AI

This is a comment left during a code review. Path: src/agents/model-fallback.ts Line: 163-175 Comment: **Fragile `meta`-field discriminant for EmbeddedPiRunResult** The `!("meta" in result)` guard is load-bearing for correctness: if a raw provider ever adds a `meta` field to its empty-content response (or if `EmbeddedPiRunResult` ever drops its `meta` field), the detection will silently misfire in the wrong direction. Since this heuristic is also completely invisible in the type system, future callers of `runWithModelFallback` have no way to know their result type needs to satisfy the guard. A more robust approach would be to either: 1. Pass an explicit `isRawProviderResult` type predicate into `runFallbackAttempt`, or 2. Require callers to mark which result types should trigger the empty-content check (e.g. a wrapper type or a config flag) The current approach is an improvement over the prior string-matched check, but it introduces a new latent coupling between `runFallbackAttempt`'s heuristic and the shape of `EmbeddedPiRunResult` that isn't enforced anywhere in the type system. How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-04-04T11:16:33Z

+  it("treats empty content array (no meta) as failure and falls back to next candidate", async () => {
+    const cfg = makeCfg({
+      agents: {
+        defaults: {
+          model: { primary: "openai/gpt-4.1-mini", fallbacks: ["fallback/ok-model"] },
+        },
+      },
+    });
+    const run = vi
+      .fn()
+      .mockResolvedValueOnce({ content: [] }) // primary: empty, no meta → EmptyResponseError
+      .mockResolvedValueOnce("ok"); // fallback succeeds
+    const result = await runWithModelFallback({
+      cfg,
+      provider: "openai",
+      model: "gpt-4.1-mini",
+      run,
+    });
+    expect(result.result).toBe("ok");
+    expect(run).toHaveBeenCalledTimes(2);
+  });


Missing test for single-candidate empty-content scenario

The new test only covers the two-candidate case (primary returns { content: [] }, fallback returns "ok"). The single-candidate path is meaningfully different: isKnownFailover is false and i === candidates.length - 1, so EmptyResponseError is thrown directly (before attempts.push and onError), bypassing throwFallbackFailureSummary.

It would be good to add a test asserting that when the only candidate returns { content: [] }, the call rejects with an EmptyResponseError (not a generic "All models failed" wrapper):

it("throws EmptyResponseError when the only candidate returns empty content", async () => { const cfg = makeCfg({ agents: { defaults: { model: { primary: "openai/gpt-4.1-mini" } } }, }); const run = vi.fn().mockResolvedValueOnce({ content: [] }); await expect( runWithModelFallback({ cfg, provider: "openai", model: "gpt-4.1-mini", run }), ).rejects.toBeInstanceOf(EmptyResponseError); expect(run).toHaveBeenCalledTimes(1); });

Prompt To Fix With AI

This is a comment left during a code review. Path: src/agents/model-fallback.test.ts Line: 1210-1230 Comment: **Missing test for single-candidate empty-content scenario** The new test only covers the two-candidate case (primary returns `{ content: [] }`, fallback returns `"ok"`). The single-candidate path is meaningfully different: `isKnownFailover` is false and `i === candidates.length - 1`, so `EmptyResponseError` is thrown directly (before `attempts.push` and `onError`), bypassing `throwFallbackFailureSummary`. It would be good to add a test asserting that when the only candidate returns `{ content: [] }`, the call rejects with an `EmptyResponseError` (not a generic "All models failed" wrapper): ```ts it("throws EmptyResponseError when the only candidate returns empty content", async () => { const cfg = makeCfg({ agents: { defaults: { model: { primary: "openai/gpt-4.1-mini" } } }, }); const run = vi.fn().mockResolvedValueOnce({ content: [] }); await expect( runWithModelFallback({ cfg, provider: "openai", model: "gpt-4.1-mini", run }), ).rejects.toBeInstanceOf(EmptyResponseError); expect(run).toHaveBeenCalledTimes(1); }); ``` How can I resolve this? If you propose a fix, please make it concise.

…mpty-result predicate - Re-throw caughtDispatchError in Telegram early-return path (P1) - Treat EmptyResponseError as known failover for observability (P2) - Replace fragile meta-field heuristic with isRawProviderResult option (P2) - Add single-candidate empty-content test (P2)

clawsweeper · 2026-04-30T00:59:36Z

Thanks for the context here. I did a careful shell check against current main, and the useful part of this older PR is already implemented there.

Close as mostly implemented on current main: the central empty/invisible terminal-result fallback path has shipped through the current model-fallback classifier, Telegram now lives in the extension path this branch does not touch, and the remaining durable final fallback semantics are owned by the open maintainer issue.

So I’m closing this older PR as already covered on main rather than keeping a mostly-duplicated branch open.

Review details

Best possible solution:

Keep the shipped classifier-based fallback implementation and route remaining durable final fallback delivery semantics through #87561 instead of rebasing this stale branch.

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

Yes, source-backed. Current main and v2026.6.6 show the model-fallback classifier path that converts empty or invisible embedded terminal results into fallback-eligible failures; I did not run live Telegram proof for this stale branch.

Is this the best way to solve the issue?

No. The branch is no longer the best fix because current main has shipped a classifier-based fallback path and Telegram moved to extensions/telegram, while the remaining delivery semantics belong in the canonical maintainer issue.

Security review:

Security review cleared: The diff only touches TypeScript fallback/Telegram dispatch code, tests, and .gitignore entries; I found no concrete security or supply-chain regression.

AGENTS.md: found and applied where relevant.

What I checked:

Current source implements result classification hook: runWithModelFallback accepts classifyResult, applies it after a successful candidate result, and converts a classification into a fallback error before continuing the candidate chain. (src/agents/model-fallback.ts:360, 8c802aa68351)
Current source classifies empty embedded terminal results: classifyEmbeddedAgentRunResultForModelFallback returns a format/empty_result classification when a GPT-5 embedded run has no visible payload, while preserving aborts, hook blocks, deliberate silent replies, visible payloads, and outbound delivery evidence. (src/agents/embedded-agent-runner/result-fallback-classifier.ts:88, 8c802aa68351)
Current callers use the classifier on agent reply paths: The auto-reply runner wires outcomePlan.classifyRunResult into runWithModelFallback so empty/invisible embedded results can advance to configured fallbacks on the active reply path. (src/auto-reply/reply/agent-runner-execution.ts:2052, 37e3e895b020)
Current Telegram fallback exists in extension implementation: Current Telegram dispatch sends a fallback when dispatch errors or non-silent delivery failures leave no delivered response; it lives under extensions/telegram rather than the obsolete src/telegram path changed by this PR. (extensions/telegram/src/bot-message-dispatch.ts:2472, 37e3e895b020)
PR Telegram path is obsolete on current main and release: Current main and v2026.6.6 no longer contain src/telegram/bot-message-dispatch.ts; Telegram implementation moved to extensions/telegram and the shim directory was removed before this review. (src/telegram/bot-message-dispatch.ts, 439c21e078f1)
Git-history provenance for current fallback implementation: Blame on the current classifier, model-fallback hook, and Telegram fallback points to c60b424 on main, a cherry-pick of the shipped release-line implementation; release tag inspection shows the same classifier source in v2026.6.6. (src/agents/embedded-agent-runner/result-fallback-classifier.ts:157, c60b42412482)

Likely related people:

Vincent Koc: Current-source blame for the model-fallback classifier hook, embedded terminal-result classifier, and current Telegram fallback points to c60b424, and the shipped release proof commit is also authored by Vincent Koc. (role: recent area contributor; confidence: high; commits: c60b42412482, 8c802aa68351; files: src/agents/model-fallback.ts, src/agents/embedded-agent-runner/result-fallback-classifier.ts, src/auto-reply/reply/agent-runner-execution.ts)
scoootscooob: Git history shows the Telegram implementation was moved from src/telegram into extensions/telegram and the old shim directory was later removed in commits authored by scoootscooob, which makes this PR's Telegram path obsolete. (role: Telegram extension migration contributor; confidence: high; commits: e5bca0832fbd, 439c21e078f1; files: src/telegram/bot-message-dispatch.ts, extensions/telegram/src/bot-message-dispatch.ts)
steipete: Git history shows the earlier Telegram dispatch-failure fallback behavior was introduced in commit 99de651 under Peter Steinberger, making him relevant for the legacy behavior line this PR tried to extend. (role: prior Telegram fallback contributor; confidence: medium; commits: 99de6515a09a; files: extensions/telegram/src/bot-message-dispatch.ts)

Codex review notes: model internal, reasoning high; reviewed against 37e3e895b020; fix evidence: release v2026.6.6, commit 8c802aa68351.

clawsweeper · 2026-05-20T10:48:47Z

ClawSweeper PR egg

🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat.

Where did the egg go?

The egg game starts only after the PR passes the real-behavior proof check.
Before that, no creature or rarity is rolled. The treat waits for real proof.
This is still just collectible flavor: proof affects review readiness, not creature quality.

openclaw-barnacle · 2026-06-05T04:52:04Z

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

clawsweeper · 2026-06-14T11:59:44Z

ClawSweeper applied the proposed close for this PR.

Action: closed this PR.
Close reason: mostly implemented on main.
Evidence: durable ClawSweeper review.

fix(agents): detect empty provider responses as failures, improve Tel…

fb3b7e1

…egram error routing

openclaw-barnacle Bot added agents Agent runtime and tooling size: M labels Apr 4, 2026

chatgpt-codex-connector Bot reviewed Apr 4, 2026

View reviewed changes

greptile-apps Bot reviewed Apr 4, 2026

View reviewed changes

100yenadmin mentioned this pull request Apr 24, 2026

[codex] Harden GPT-5.4 runtime paths #70743

Merged

clawsweeper Bot mentioned this pull request Apr 28, 2026

fix(channels): extend user-visible error fallback to Discord, Slack, Signal, WhatsApp #60812

Closed

clawsweeper Bot added the mantis: telegram-visible-proof Mantis should capture Telegram visible proof. label May 11, 2026

openclaw-barnacle Bot added the triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. label May 11, 2026

clawsweeper Bot added the merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. label May 21, 2026

osolmaz mentioned this pull request May 28, 2026

Define durable final fallback delivery semantics across channels #87561

Open

openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Jun 5, 2026

openclaw-barnacle Bot removed the stale Marked as stale due to inactivity label Jun 6, 2026

clawsweeper Bot had a problem deploying to qa-live-shared June 14, 2026 11:15 Failure

clawsweeper Bot closed this Jun 14, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(agents): detect empty provider responses as failures, improve Telegram error routing#60830

fix(agents): detect empty provider responses as failures, improve Telegram error routing#60830
garnetlyx wants to merge 2 commits into
openclaw:mainfrom
garnetlyx:fix/empty-response-silent-failures

garnetlyx commented Apr 4, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Apr 4, 2026

Uh oh!

greptile-apps Bot commented Apr 4, 2026 •

edited

Loading

Comments Outside Diff (1)

Uh oh!

greptile-apps Bot Apr 4, 2026

Uh oh!

greptile-apps Bot Apr 4, 2026

Uh oh!

clawsweeper Bot commented Apr 30, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented May 20, 2026

Uh oh!

openclaw-barnacle Bot commented Jun 5, 2026

Uh oh!

clawsweeper Bot commented Jun 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

garnetlyx commented Apr 4, 2026

Summary

Related

Test plan

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Apr 4, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot commented Apr 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 4/5

Comments Outside Diff (1)

Uh oh!

greptile-apps Bot Apr 4, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Apr 4, 2026

Choose a reason for hiding this comment

Uh oh!

clawsweeper Bot commented Apr 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

clawsweeper Bot commented May 20, 2026

Uh oh!

openclaw-barnacle Bot commented Jun 5, 2026

Uh oh!

clawsweeper Bot commented Jun 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

greptile-apps Bot commented Apr 4, 2026 •

edited

Loading

clawsweeper Bot commented Apr 30, 2026 •

edited

Loading