Skip to content

fix: _KeyRotator index bounds check#9040

Merged
Soulter merged 1 commit into
AstrBotDevs:masterfrom
fan-67:master
Jun 27, 2026
Merged

fix: _KeyRotator index bounds check#9040
Soulter merged 1 commit into
AstrBotDevs:masterfrom
fan-67:master

Conversation

@fan-67

@fan-67 fan-67 commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

[Bug] _KeyRotator IndexError when key list shrinks

What

_KeyRotator.get() in astrbot/core/tools/web_search_tools.py raises IndexError when the configured key list has fewer entries than the internal index counter expects.

Same bug affects all four rotators: _TAVILY_KEY_ROTATOR, _BOCHA_KEY_ROTATOR, _BRAVE_KEY_ROTATOR, _FIRECRAWL_KEY_ROTATOR.

Why

_KeyRotator is a module-level singleton. self.index persists across calls but only resets on bot restart. If someone configures 2 keys (index advances to 1), then later removes one (1 key left), the if not keys guard passes but keys[self.index] still blows up.

Fix

One-line bounds check before accessing keys[self.index]:

async with self.lock:
    if self.index >= len(keys):
        self.index = 0
    key = keys[self.index]
    ...

Tested

  • index=1, keys=["a"] → was IndexError, now returns "a" and resets index to 0
  • Normal rotation with 2 keys still works correctly
  • Single key with index=0 unchanged

Closes #9039

Summary by Sourcery

Bug Fixes:

  • Prevent IndexError in key rotator when the internal index exceeds the current number of configured keys.

@dosubot dosubot Bot added size:XS This PR changes 0-9 lines, ignoring generated files. area:core The bug / feature is about astrbot's core, backend labels Jun 26, 2026

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request introduces a bounds check on self.index before accessing the keys list to prevent potential out-of-bounds errors. The reviewer pointed out that the async with self.lock: block is redundant because the synchronous code within it executes atomically in a single-threaded asyncio event loop, and suggested removing the lock to simplify the code and avoid unnecessary overhead.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines 75 to 80
async with self.lock:
if self.index >= len(keys):
self.index = 0
key = keys[self.index]
self.index = (self.index + 1) % len(keys)
return key

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.

medium

In a single-threaded asyncio event loop, synchronous code blocks (those without any await statements) are executed atomically and will not be interrupted by other coroutines. Therefore, the async with self.lock: block is redundant and can be safely removed to simplify the code and avoid unnecessary locking overhead.

        if self.index >= len(keys):
            self.index = 0
        key = keys[self.index]
        self.index = (self.index + 1) % len(keys)
        return key
References
  1. In a single-threaded asyncio event loop, synchronous functions (code blocks without 'await') are executed atomically and will not be interrupted by other coroutines. Therefore, they are safe from race conditions when modifying shared state within that block.

@sourcery-ai sourcery-ai Bot 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.

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label Jun 27, 2026
@Soulter Soulter merged commit 534ad0c into AstrBotDevs:master Jun 27, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:core The bug / feature is about astrbot's core, backend lgtm This PR has been approved by a maintainer size:XS This PR changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]KeyRotator 在 key 列表缩减后 IndexError

2 participants