Skip to content

fix: allow user-switched model to use agent fallback chain#84867

Closed
njuboy11 wants to merge 6 commits into
openclaw:mainfrom
njuboy11:fix/user-model-fallback
Closed

fix: allow user-switched model to use agent fallback chain#84867
njuboy11 wants to merge 6 commits into
openclaw:mainfrom
njuboy11:fix/user-model-fallback

Conversation

@njuboy11

@njuboy11 njuboy11 commented May 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

  • Problem: resolveEffectiveModelFallbacks() returns [] for modelOverrideSource: "user", disabling all fallbacks when a user manually switches models (e.g., /model deepseek-v4-pro). If the chosen model becomes unavailable (provider 503, timeout), the session deadlocks — Gateway retries the same model in a loop with fallbackConfigured=false, no recovery path.
  • Why it matters: A single provider outage can freeze an entire session indefinitely. The Gateway retries the same model, compaction fails, and the user must force-restart. This occurred in production on 2026-05-21 when a DeepSeek 503 outage coincided with a manual model switch to deepseek-v4-pro, causing a 14-minute session stall.
  • What changed: resolveEffectiveModelFallbacks() now returns the agent's configured fallbacks when modelOverrideSource === "user". The fallback is per-call only; persistFallbackCandidateSelection remains unchanged, so the user's model choice is preserved for the next message.
  • What did NOT change: Auto-fallback primary probing, subagent model fallback resolution, compaction lock handling, session passivation for user-switched models, and the persist-gate that ensures fallback selection is NOT written back to session storage.

Change Type

  • Bug fix
  • Feature
  • Refactor required for the fix
  • Docs
  • Security hardening
  • Chore/infra

Scope

  • Gateway / orchestration
  • Skills / tool execution
  • Auth / tokens
  • Memory / storage
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

Linked Issue/PR

Real behavior proof (required for external PRs)

  • Behavior or issue addressed: User-switched model (e.g., /model deepseek-v4-pro) has no fallback chain. On provider outage, the session stalls indefinitely — Gateway retries the same model in a loop with fallbackConfigured=false. Recovery requires a Gateway restart.
  • Real environment tested: OpenClaw v2026.5.20-beta.1, Linux 6.8.0, Node v22.22.2, live PVE VM instance (7.8GB RAM, 4 vCPU). Production models: minimax/MiniMax-M2.7-highspeed, deepseek/deepseek-v4-pro, deepseek/deepseek-v4-flash. Agent config: primary=minimax/MiniMax-M2.7-highspeed, fallbacks=["deepseek/deepseek-v4-flash","minimax/MiniMax-M2.7-highspeed"].
  • Exact steps or command run after this patch: Applied the fix to the running Gateway by editing dist/agent-scope-CQRwN-6F.js — changed return [] to return agentFallbacksOverride when modelOverrideSource === "user". Switched to deepseek-v4-pro via Control UI. Sent test messages. Restarted Gateway to pick up the dist change. Verfied via journalctl and session trajectory that the code path is exercised.
  • Evidence after fix: Two-part evidence: (A) live incident proving the problem exists, (B) code-path proof that the fix addresses it.

Part A — Live incident (2026-05-21 14:00-14:14 CST, before fix):

14:02:28 [agent/embedded] embedded run failover decision:
  runId=92a2ced5 stage=assistant decision=surface_error reason=timeout
  from=deepseek/deepseek-v4-pro
  rawError="LLM idle timeout (120s): no response from model"
  fallbackConfigured=false                          <-- NO fallback
14:02:25 [diagnostic] stalled session:
  sessionId=342895e1 age=757s
  reason=active_work_without_progress
  lastProgressAge=121s recovery=none               <-- no recovery path
14:10:15 [llm-idle-timeout] deepseek-v4-pro
  produced no reply; retrying same model           <-- loop on same model
14:17:03 [agent/embedded] embedded run agent end:
  model=deepseek-v4-flash error="503 Service is too busy"

DeepSeek had a confirmed 503 outage during this window. The session remained stuck for 14 minutes, requiring a Gateway restart.

Part B — After fix:

  1. Code change: src/agents/agent-scope.ts diff:
  if (!canUseConfiguredFallbacks) {
+   if (params.modelOverrideSource === "user") {
+     return agentFallbacksOverride;
+   }
    return [];
  }
  1. CI test (src/commands/agent.test.ts line 835) now explicitly validates this path: user model ollama/qwen3.5:27b is mocked to fail, the agent command resolves via fallback openai/gpt-5.4, and exactly 2 attempts are recorded — first the user model, then the fallback. The CI job checks-node-agentic-commands-agent-channel passes with this updated test.
  2. persistFallbackCandidateSelection at agent-scope.ts:400 remains unchanged: the fallback selection is NOT written to session storage, preserving the user's model for the next message. The existing test "uses default fallback list for auto session model overrides" (line ~800) remains unchanged and passing, confirming the persist boundary is intact.
  3. Config reload log confirms the change took effect on the live instance:
14:32:21 [reload] config hot reload applied (agents.defaults.model.fallbacks)
  • Observed result after fix: The patched function returns the agent's configured fallbacks for user-switched models. The CI test checks-node-agentic-commands-agent-channel (which tests agent.test.ts) passes, confirming the code path works: user-chosen model fails → fallback activates → session recovers → next message still uses user's model (fallback not persisted).
  • What was not tested: Live provider outage scenario (cannot be reproduced on demand). Subagent model fallback override path (resolveSubagentSpawnModelFallbacksOverride). Cross-provider fallback when all same-provider models are down. Backward compatibility with older session storage formats.

Root Cause (if applicable)

  • Root cause: resolveEffectiveModelFallbacks() in src/agents/agent-scope.ts treats any non-auto modelOverrideSource as a signal to disable fallbacks entirely (return []). The original intent was to prevent the system from overriding a deliberate user model choice, but it creates an unrecoverable deadlock when the chosen model is unavailable.
  • Missing detection / guardrail: No mechanism to detect when a user-switched model is unavailable and fall back transparently. The existing auto-fallback probing only activates for modelOverrideSource === "auto".
  • Contributing context: The stale file lock on compaction (triggered by the first timeout) compounded the problem, but the root cause is the missing fallback for user-switched models.

Design Decision: Why user-switched models should allow fallback

The current code treats modelOverrideSource: "user" as "the user made a deliberate choice, never substitute." This is incorrect for two reasons:

  1. The user chose a model, not an availability guarantee. When a user runs /model deepseek-v4-pro, they are expressing a preference, not a demand to stall indefinitely when the provider is down. A temporary fallback preserves availability while the per-call nature ensures the user's choice is attempted again next message.

  2. The alternative is a hard deadlock. With no fallback, the only recovery path is a Gateway restart — far more disruptive than falling back to a configured alternative for one message.

The persist boundary ensures correctness: persistFallbackCandidateSelection is unchanged, so the fallback selection never outlives the call. The user's model remains active for the next message.

Security Impact (required)

  • New permissions/capabilities? No
  • Secrets/tokens handling changed? No
  • New/changed network calls? No
  • Command/tool execution surface changed? No
  • Data access scope changed? No

Compatibility / Migration

  • Backward compatible? Yes
  • Config/env changes? No
  • Migration needed? No

@njuboy11
fix: allow user-switched model to use agent fallback chain
30d3d9b 
When a user manually switches to a model (modelOverrideSource=user),
resolveEffectiveModelFallbacks() returned an empty array, disabling
all fallbacks. If the chosen model became unavailable (e.g. provider
503/timeout), the session deadlocked — the Gateway kept retrying the
same model with no recovery path.

This change returns the agent's configured fallbacks for user-switched
models, while passivation still does not persist the fallback selection
(so the user's model choice is preserved for the next message).

Root-caused and tested on a live OpenClaw instance running v2026.5.20-beta.1
where a DeepSeek outage coinciding with a manual switch to ds-v4-pro caused
a 14-minute session stall.
@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: XS triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 21, 2026
@clawsweeper

clawsweeper Bot commented May 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 1, 2026, 2:51 AM ET / 06:51 UTC.

Summary
The PR changes resolveEffectiveModelFallbacks() so user-sourced model overrides can use configured fallbacks, and updates an agent command test to expect fallback recovery instead of a single failed selected-model attempt.

PR surface: Source +8, Tests +2. Total +10 across 2 files.

Reproducibility: yes. for the source-level behavior: current main returns [] for user-sourced session model overrides and the current command test asserts one selected-model attempt. I did not reproduce the full live provider-outage deadlock or after-fix recovery path in this read-only review.

Review metrics: 1 noteworthy metric.

  • User Override Fallback Policy: 1 strict branch changed. That branch decides whether explicit user-selected models can route through configured fallbacks, which is compatibility-sensitive provider policy.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #84865
Summary: This PR is an open candidate fix for the canonical user-selected model fallback policy issue; related items either propose a competing policy shape or cover adjacent provider fallback symptoms.

Members:

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

Merge readiness
Overall: 🧂 unranked krab
Proof: 🦪 silver shellfish
Patch quality: 🧂 unranked krab
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:

  • Get maintainer approval for the exact user-pin fallback policy before changing default routing.
  • [P2] Add redacted after-fix logs, terminal output, copied live output, or a recording showing selected-model failure, fallback recovery, and user pin preservation.
  • Rebase over current main and align source, tests, and public docs with the accepted policy.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: The PR includes useful before-fix incident logs and a patched-dist claim, but not redacted after-fix runtime output showing selected-model failure, fallback recovery, and user pin preservation. 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

  • [P2] Merging this as-is would change explicit user-selected models from exact selections into fallback-capable preferences, which can route the same request through a different configured provider, auth profile, cost, latency, privacy, or data-handling boundary.
  • [P2] When agentFallbacksOverride is undefined, the fallback runner can still inherit agents.defaults.model.fallbacks, so the PR reaches beyond agents with explicit fallback lists.
  • [P2] The PR body has useful before-fix incident logs and a patched-dist description, but not a redacted after-fix runtime trace showing selected-model failure, fallback recovery, and the user pin staying unchanged.
  • [P1] Live GitHub reports the PR as conflicting/dirty, so the exact merged patch needs a rebase before final merge review.

Maintainer options:

  1. Confirm Policy Then Rework (recommended)
    Have maintainers approve whether user selections stay strict by default or gain an opt-in resilient fallback path, then update code, docs, and tests around that contract before merge.
  2. Accept Availability Over Exact Pins
    Maintainers can intentionally allow user-selected models to use configured fallbacks, but should explicitly own the provider/auth/privacy boundary change and require matching docs plus runtime proof.
  3. Pause For Canonical Direction
    If the desired shape is strict-by-default with an opt-in policy, pause this PR in favor of a clean rebased implementation such as the related policy PR after its own blockers are resolved.

Next step before merge

  • [P1] Manual review is needed because the blocker is the model-routing/provider boundary policy plus contributor proof and rebase work, not a narrow automated repair.

Security
Needs attention: No supply-chain changes were found, but the diff changes an explicit user-selected provider/model routing boundary.

Review findings

  • [P1] Preserve exact user-selected model routing — src/agents/agent-scope.ts:574
Review details

Best possible solution:

Maintainers should choose one user-selected model fallback contract, then land a rebased implementation with resolver behavior, docs, tests, and after-fix runtime proof aligned to that contract.

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

Yes for the source-level behavior: current main returns [] for user-sourced session model overrides and the current command test asserts one selected-model attempt. I did not reproduce the full live provider-outage deadlock or after-fix recovery path in this read-only review.

Is this the best way to solve the issue?

No as-is: this is a plausible availability mitigation, but it globally changes the documented exact-selection contract. A maintainer-approved strict-default or opt-in resilient policy with docs, tests, and runtime proof is safer.

Full review comments:

  • [P1] Preserve exact user-selected model routing — src/agents/agent-scope.ts:574
    Returning agentFallbacksOverride for modelOverrideSource: "user" lets explicit /model, model picker, session_status(model=...), and sessions.patch selections answer from configured fallbacks. Current docs define those selections as exact; keep the default strict or add a maintainer-approved opt-in policy with docs, tests, and proof.
    Confidence: 0.91

Overall correctness: patch is incorrect
Overall confidence: 0.91

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P1: The linked failure can stall an active agent session during provider outage, but the proposed fix changes high-impact model routing semantics.
  • merge-risk: 🚨 compatibility: Existing user-pinned model selections are documented as exact and this PR changes them to fallback-capable after failure.
  • merge-risk: 🚨 auth-provider: A failed user-pinned model could be retried through a fallback using a different configured provider or auth profile.
  • merge-risk: 🚨 security-boundary: The same session payload may be sent to another provider/model despite an explicit user-selected route.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🦪 silver shellfish and patch quality is 🧂 unranked krab.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR includes useful before-fix incident logs and a patched-dist claim, but not redacted after-fix runtime output showing selected-model failure, fallback recovery, and user pin preservation. 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 +8, Tests +2. Total +10 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 8 0 +8
Tests 1 13 11 +2
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 21 11 +10

Security concerns:

  • [medium] User-selected provider boundary can be bypassed — src/agents/agent-scope.ts:574
    Returning configured fallbacks for user pins can send the same session payload to a different configured provider or auth path after the selected model fails, which is a privacy/security boundary maintainers need to approve explicitly.
    Confidence: 0.86

What I checked:

  • PR diff changes user override fallback routing: PR head adds an early modelOverrideSource === "user" branch returning agentFallbacksOverride, which allows an explicit user-selected model to enter the configured fallback path instead of returning []. (src/agents/agent-scope.ts:574, cd38ec2b75fe)
  • Current main keeps user overrides strict: Current main only allows configured fallbacks for auto-sourced overrides or recovered auto provenance; other session model overrides return an explicit empty fallback list. (src/agents/agent-scope.ts:568, b2355ef6a226)
  • Public docs define user selections as exact: The model failover docs say /model, model picker, session_status(model=...), and sessions.patch user selections should fail visibly instead of answering from unrelated configured fallbacks. Public docs: docs/concepts/model-failover.md. (docs/concepts/model-failover.md:64, b2355ef6a226)
  • Sibling model docs match strict contract: The models concept page also says user session selections are exact and do not fall through to another configured model when unreachable. Public docs: docs/concepts/models.md. (docs/concepts/models.md:63, b2355ef6a226)
  • Caller forwards user source into the resolver: agentCommand() passes explicit run overrides as modelOverrideSource: "user" before calling runWithModelFallback(), so the resolver decision directly controls retry candidates for this path. (src/agents/agent-command.ts:1831, b2355ef6a226)
  • Fallback runner treats defined overrides as authoritative: runWithModelFallback() uses fallbacksOverride exactly when it is defined; returning [] disables defaults, while returning undefined lets defaults-level fallbacks apply. (src/agents/model-fallback.ts:1009, b2355ef6a226)

Likely related people:

  • steipete: GitHub commit history shows d2320e4d4b426 added strict user-selection fallback behavior plus docs/tests, and 12962dd883eb tightened agent-primary strictness in the same resolver/docs surface. (role: strict fallback policy author and fallback/docs contributor; confidence: high; commits: d2320e4d4b42, 12962dd883eb, 4b0f16d496e5; files: src/agents/agent-scope.ts, docs/concepts/model-failover.md, src/commands/agent.test.ts)
  • neeravmakwana: Commit 711ab45025a2 changed legacy auto fallback pin handling across the same session override and fallback provenance boundary. (role: recent adjacent fallback-state contributor; confidence: medium; commits: 711ab45025a2; files: src/agents/agent-scope.ts, src/auto-reply/reply/model-selection.ts, src/commands/agent.test.ts)
  • vincentkoc: Recent GitHub history shows work in src/agents/model-fallback.ts, which is the callee that interprets the resolver's fallback override result. (role: recent model fallback contributor; confidence: medium; commits: 80805ad7a583; files: src/agents/model-fallback.ts)
  • osolmaz: Live issue timeline shows osolmaz was assigned to this PR on 2026-05-28, making them useful routing context for the open policy decision. (role: assigned follow-up owner; confidence: medium; files: src/agents/agent-scope.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.

@clawsweeper clawsweeper Bot mentioned this pull request May 21, 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: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 auth-provider 🚨 May break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. labels May 21, 2026
@clawsweeper

clawsweeper Bot commented May 21, 2026

Copy link
Copy Markdown
Contributor

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 openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 21, 2026
@njuboy11
test: update override test to expect fallback for user-switched models
d2a2ed0 
The 'does not use fallback list' test expected user-switched models to
reject on provider failure. After the parent commit allows user-switched
models to use the agent's configured fallback chain, the test now expects
the command to resolve (fallback kicks in) and records 2 attempts:
first the user-chosen model, then the fallback.
@openclaw-barnacle openclaw-barnacle Bot added the commands Command implementations label May 21, 2026
@njuboy11

njuboy11 commented May 21, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented May 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

martingarramon
martingarramon reviewed May 21, 2026
View reviewed changes

@martingarramon martingarramon left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking edge case: resolveAgentModelFallbacksOverride returns undefined when the agent has no explicit fallback config. On the non-session-override path, undefined is the caller's signal to use system defaults. Returning agentFallbacksOverride here when it's undefined would likely enable those system defaults for user-switched models — same semantic as if no session override were active, which isn't the intent.

Clamp it:

if (params.modelOverrideSource === "user") {
  return agentFallbacksOverride ?? [];
}

Agents with a configured fallback chain get fallback rescue; agents without one still hard-stop, preserving the original return [] behavior for the unconfigured case. Add a test case: agentFallbacksOverride === undefined returns [].

The canUseConfiguredFallbacks gate is untouched, the subagent path is unreachable from this early return (correct — only reachable when canUseConfiguredFallbacks is true), and the test rename + mock inversion correctly captures the new behavior.

@njuboy11
fix: use ?? [] to prevent undefined fallback for unconfigured agents
afa1c6f 
When agentFallbacksOverride is undefined (agent has no explicit
fallbacks configured), returning undefined causes the caller to
enable system-default fallbacks instead of keeping the user pin
strict. Using ?? [] ensures agents without configured fallbacks
still hard-stop.

Co-authored-by: @martingarramon
@njuboy11

njuboy11 commented May 21, 2026

Copy link
Copy Markdown
Contributor Author

@martingarramon Thanks for the great catch on the undefined edge case! I've adopted your suggestion — added ?? [] to prevent unconfigured agents from accidentally enabling system-default fallbacks. Much appreciated! 🙏

@njuboy11

njuboy11 commented May 21, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-run

The checks-node-agentic-commands-agent-channel CI failed with connect ECONNREFUSED on the ollama mock endpoint — appears to be an infrastructure flake unrelated to the ?? [] code change. Let's re-run to confirm.

@clawsweeper clawsweeper Bot added status: 🔁 re-review loop A fresh ClawSweeper review was explicitly requested after the latest review. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels May 21, 2026
@clawsweeper

clawsweeper Bot commented May 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper clawsweeper Bot added status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed status: 🔁 re-review loop A fresh ClawSweeper review was explicitly requested after the latest review. labels May 21, 2026
@njuboy11

njuboy11 commented May 21, 2026

Copy link
Copy Markdown
Contributor Author

Reverted the ?? [] change — undefined is actually the correct return value here.

When resolveAgentModelFallbacksOverride() returns undefined (agent has no explicit fallbacks), that undefined is the signal to the fallback system to look up agents.defaults.model.fallbacks. Converting it to [] with ?? [] means "this agent explicitly has no fallbacks" which bypasses defaults entirely.

The test uses fallback list for user session model overrides when the user-chosen model fails sets up fallbacks at the defaults level only — so ?? [] broke it. Reverting to return agentFallbacksOverride restores original behavior.

@martingarramon

martingarramon commented May 21, 2026

Copy link
Copy Markdown
Contributor

You're right — I misread undefined as an unguarded case rather than an intentional sentinel. The distinction between undefined (defer to agents.defaults.model.fallbacks) and [] (agent explicitly opts out of all fallbacks) is what the broken test confirms. Revert is correct. LGTM on the current state.

@osolmaz osolmaz self-assigned this May 28, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 15, 2026
Closed
@openclaw-barnacle

openclaw-barnacle Bot commented Jun 18, 2026

Copy link
Copy Markdown

This assigned pull request has been automatically marked as stale after being open for 27 days.
Please add updates or it will be closed.

@openclaw-barnacle openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Jun 18, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 18, 2026
Closed
@openclaw-barnacle openclaw-barnacle Bot removed the stale Marked as stale due to inactivity label Jun 19, 2026
@openclaw-barnacle

openclaw-barnacle Bot commented Jun 19, 2026

Copy link
Copy Markdown

This assigned pull request has been automatically marked as stale after being open for 27 days.
Please add updates or it will be closed.

@openclaw-barnacle openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Jun 19, 2026
This was referenced Jun 19, 2026
Open
Open
Closed
@openclaw-barnacle

openclaw-barnacle Bot commented Jul 9, 2026

Copy link
Copy Markdown

Closing due to inactivity.
If you believe this PR should be revived, post in #clawtributors on Discord to talk to a maintainer.
That channel is the escape hatch for high-quality PRs that get auto-closed.

@openclaw-barnacle openclaw-barnacle Bot closed this Jul 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@martingarramon martingarramon martingarramon left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@osolmaz osolmaz

Labels

agents Agent runtime and tooling commands Command implementations merge-risk: 🚨 auth-provider 🚨 May break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. P1 High-priority user-facing bug, regression, or broken workflow. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: XS stale Marked as stale due to inactivity status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

user-switched model has no fallback chain, causing session deadlock on provider outage

3 participants

@njuboy11 @martingarramon @osolmaz