Skip to content

fix(switch_model): guard _fallback_chain access on partially-constructed agents#14731

Closed
AndreKurait wants to merge 1 commit into
NousResearch:mainfrom
AndreKurait:fix/fallback-chain-getattr-guard
Closed

fix(switch_model): guard _fallback_chain access on partially-constructed agents#14731
AndreKurait wants to merge 1 commit into
NousResearch:mainfrom
AndreKurait:fix/fallback-chain-getattr-guard

Conversation

@AndreKurait

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes an AttributeError: 'AIAgent' object has no attribute '_fallback_chain' in AIAgent.switch_model() when called on a partially-constructed agent. Three-line defensive guard using getattr(..., default).

Why?

switch_model() prunes self._fallback_chain when the user swaps primary providers (openrouter → anthropic, etc.). The original code read self._fallback_chain directly:

self._fallback_chain = [
    entry for entry in self._fallback_chain  # AttributeError here
    if (entry.get("provider") or "").strip().lower() not in {old_norm, new_norm}
]

If __init__ hasn't run (e.g. tests using AIAgent.__new__(AIAgent) with __init__ patched to return_value=None), _fallback_chain doesn't exist yet and the attribute read raises.

Reproduction

pytest tests/agent/test_minimax_provider.py::TestMinimaxSwitchModelCredentialGuard::test_switch_to_minimax_does_not_resolve_anthropic_token

Without this fix:

AttributeError: 'AIAgent' object has no attribute '_fallback_chain'
run_agent.py:2042: AttributeError
FAILED tests/agent/test_minimax_provider.py::...

With this fix:

41 passed in 4.32s

(That test was clearly written against an earlier version where the pruning block didn't exist, then the pruning was added later without updating the test fixture. Rather than force every caller to pre-populate _fallback_chain, this PR makes the consumer defensive.)

Changes

run_agent.py:2041-2044

     if old_norm and new_norm and old_norm != new_norm:
+        # getattr guards against partially-constructed agents (e.g. tests
+        # using AIAgent.__new__ to isolate switch_model behavior).
+        existing_chain = getattr(self, "_fallback_chain", []) or []
         self._fallback_chain = [
-            entry for entry in self._fallback_chain
+            entry for entry in existing_chain
             if (entry.get("provider") or "").strip().lower() not in {old_norm, new_norm}
         ]

Which type of PR is this?

  • Bug fix (non-breaking change which fixes an issue)

How has this been tested?

pytest tests/agent/test_minimax_provider.py -q
# 41 passed in 4.32s

Risk

Minimal:

  • getattr(obj, name, default) is idiomatic Python with zero runtime cost when the attribute exists.
  • or [] guards the unlikely case where someone manually set the attribute to None.
  • No behaviour change for production code paths — _fallback_chain is always set by __init__ in the normal agent construction path.

Checklist

  • My changes follow the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

…ted agents

## Problem

`AIAgent.switch_model()` unconditionally reads ``self._fallback_chain``
when pruning the fallback chain after a provider swap (run_agent.py:2041).

If an agent is partially constructed — e.g. tests that use
``AIAgent.__new__(AIAgent)`` with ``__init__`` mocked, to isolate
``switch_model`` behavior — ``_fallback_chain`` may not exist yet,
causing:

    AttributeError: 'AIAgent' object has no attribute '_fallback_chain'

Reproduction: `pytest tests/agent/test_minimax_provider.py::\
TestMinimaxSwitchModelCredentialGuard::\
test_switch_to_minimax_does_not_resolve_anthropic_token`

Without this fix, the test fails with the above AttributeError at
run_agent.py:2042 because the test patches ``AIAgent.__init__`` to
`return_value=None` and never sets ``_fallback_chain``.

## Fix

Replace the direct attribute read with ``getattr(self, "_fallback_chain",
[]) or []``. This preserves the existing behaviour for fully-initialized
agents (the attribute is always a list in that case) while keeping
partially-constructed test fixtures working.

## Changes

- `run_agent.py:2041-2044`
  - Add ``existing_chain = getattr(self, "_fallback_chain", []) or []``
    before the list-comprehension.
  - Switch the comprehension to iterate ``existing_chain``.
  - 3-line change, no behavioural difference for production code paths.

## Testing

Verified the test failure reproduces without the fix and passes with it:

  # Without fix:
  AttributeError: 'AIAgent' object has no attribute '_fallback_chain'
  FAILED tests/agent/test_minimax_provider.py::\
    TestMinimaxSwitchModelCredentialGuard::\
    test_switch_to_minimax_does_not_resolve_anthropic_token

  # With fix:
  PASSED tests/agent/test_minimax_provider.py -q
  # 41 passed in 4.32s

## Risk

Minimal. `getattr(obj, name, default)` is idiomatic Python for defensive
attribute access and has zero runtime cost when the attribute exists.
The `or []` guards the (extremely unlikely) case where someone manually
set the attribute to None.

Signed-off-by: Andre Kurait <andrekurait@gmail.com>
@teknium1

Copy link
Copy Markdown
Contributor

Thanks @AndreKurait. Closing — superseded by @LeonSGP43's #14867 which landed in #15134 (commit a9fd8d7).

Both PRs defend against missing _fallback_chain during switch_model(), but #14867 is more thorough: it unconditionally assigns self._fallback_chain = fallback_chain at the end of the function (whether or not the prune branch fired), so both the partially-constructed-agent case AND the old_provider == new_provider no-op case are covered. Your getattr(self, "_fallback_chain", []) pattern only guarded the read path.

You both spotted the same class of bug — thanks for the careful report.

@teknium1 teknium1 closed this Apr 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent loop, run_agent.py, prompt builder P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants