Refactor bot management and introduce caching mechanism

[m-xim]

---

Summary by CodeRabbit

• New Features

  • Token-based multi-bot resolution with lazy caching and a read-only bots view; ability to remove cached bots
  • Secret-token header constant introduced for webhook verification

• Breaking Changes

  • Security APIs now accept a bot token string (not a bot object)
  • Routing API renamed to webhook_url (now async) and token extraction renamed to resolve_token
  • Engine constructor param renamed bot_settings → bot_config; custom engines inherit from WebhookEngine

• Documentation

  • Updated custom-engine docs and added a “Request processing” subsection

• Chores

  • CI: added a type-checking step to tests

[m-xim]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

♻️ Duplicate comments (1)

src/aiogram_webhook/engines/token.py (1)

57-58: ⚠️ Potential issue | 🟠 Major

Handle malformed tokens consistently for set_webhook path.

Line 58 can raise on malformed token parsing, and Line 94 calls this path without normalization. This makes set_webhook() error behavior inconsistent with request handling (which already maps invalid tokens to a controlled response).

💡 Suggested fix

-from aiogram.utils.token import extract_bot_id
+from aiogram.utils.token import TokenValidationError, extract_bot_id
@@
-    def _get_bot_by_token(self, token: str) -> Bot:
-        bot_id = extract_bot_id(token)
+    def _get_bot_by_token(self, token: str) -> Bot | None:
+        try:
+            bot_id = extract_bot_id(token)
+        except (TokenValidationError, ValueError):
+            return None
         existing_bot = self._bots.get(bot_id)
@@
-        bot = self._get_bot_by_token(token=token)
+        bot = self._get_bot_by_token(token=token)
+        if bot is None:
+            raise ValueError("Invalid bot token")

In aiogram 3.14.0, what exceptions can `aiogram.utils.token.extract_bot_id(token)` raise for malformed token input?

Also applies to: 94-95

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4bbab598-2de4-4488-9377-67dddc16c3bc

📥 Commits

Reviewing files that changed from the base of the PR and between 66ecb5d and dc83170.

📒 Files selected for processing (8)

• src/aiogram_webhook/engines/base.py
• src/aiogram_webhook/engines/simple.py
• src/aiogram_webhook/engines/token.py
• src/aiogram_webhook/routing/base.py
• src/aiogram_webhook/routing/path.py
• src/aiogram_webhook/routing/query.py
• src/aiogram_webhook/routing/static.py
• tests/test_routing.py

🚧 Files skipped from review as they are similar to previous changes (1)

• src/aiogram_webhook/engines/simple.py

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/aiogram_webhook/engines/token.py (1)

124-131: ⚠️ Potential issue | 🟠 Major

Drain in-flight background tasks before closing the shared session.

Line 128 may close self._session while background update tasks are still running, which can fail in-flight API calls during shutdown and drop updates.

Proposed fix

+import asyncio
 from types import MappingProxyType
 from typing import TYPE_CHECKING, Any
@@
     async def on_shutdown(self, app: Any, *args, **kwargs) -> None:  # noqa: ARG002
         workflow_data = self._build_workflow_data(app=app, bots=set(self._bots.values()), **kwargs)
         await self.dispatcher.emit_shutdown(**workflow_data)
 
+        pending_tasks = tuple(self._background_feed_update_tasks)
+        if pending_tasks:
+            await asyncio.gather(*pending_tasks, return_exceptions=True)
+
         if self.bot_config.session is None:
             await self._session.close()
 
         self._bots.clear()

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/aiogram_webhook/engines/token.py` around lines 124 - 131, on_shutdown
currently closes self._session immediately which can abort in-flight background
update tasks; before calling await self._session.close() in on_shutdown,
drain/await your background tasks (e.g., any task list like
self._background_tasks, self._pending_tasks or a task group used for updates) by
cancelling if needed and awaiting them with asyncio.gather(...,
return_exceptions=True) or awaiting the task-group shut down, then only close
self._session and clear self._bots; update on_shutdown to locate the engine's
background task container (reference on_shutdown, self._session and the
background task attribute/name used in this class) and wait for those tasks to
finish/settle before closing the shared session.

🧹 Nitpick comments (1)

src/aiogram_webhook/engines/base.py (1)

132-135: Capture background task exceptions for observability.

Line 133 schedules fire-and-forget work, but done-callback cleanup on Line 135 does not surface task failures with context. Consider consuming/logging exceptions in a dedicated done callback.

Suggested refinement

+import logging
+
+logger = logging.getLogger(__name__)
+
     async def _handle_request_background(self, bot: Bot, update: dict[str, Any]):
         feed_update_task = asyncio.create_task(self._background_feed_update(bot=bot, update=update))
         self._background_feed_update_tasks.add(feed_update_task)
         feed_update_task.add_done_callback(self._background_feed_update_tasks.discard)
+        feed_update_task.add_done_callback(self._log_background_task_failure)
 
         return self.web_adapter.create_json_response(status=200, payload={})
+
+    `@staticmethod`
+    def _log_background_task_failure(task: asyncio.Task[Any]) -> None:
+        try:
+            task.result()
+        except asyncio.CancelledError:
+            pass
+        except Exception:
+            logger.exception("Background webhook update task failed")

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/aiogram_webhook/engines/base.py` around lines 132 - 135,
_handle_request_background currently creates a fire-and-forget task
(feed_update_task) and only uses _background_feed_update_tasks.discard as the
done callback, which hides task exceptions; change the done callback to a small
wrapper that 1) removes the task from _background_feed_update_tasks and 2)
checks task.exception() and surfaces it (e.g., call self.logger.exception(...)
or pass to the event loop's exception handler) so any errors in
_background_feed_update are logged/observed with context instead of being
silently dropped.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/aiogram_webhook/engines/token.py`:
- Around line 124-131: on_shutdown currently closes self._session immediately
which can abort in-flight background update tasks; before calling await
self._session.close() in on_shutdown, drain/await your background tasks (e.g.,
any task list like self._background_tasks, self._pending_tasks or a task group
used for updates) by cancelling if needed and awaiting them with
asyncio.gather(..., return_exceptions=True) or awaiting the task-group shut
down, then only close self._session and clear self._bots; update on_shutdown to
locate the engine's background task container (reference on_shutdown,
self._session and the background task attribute/name used in this class) and
wait for those tasks to finish/settle before closing the shared session.

---

Nitpick comments:
In `@src/aiogram_webhook/engines/base.py`:
- Around line 132-135: _handle_request_background currently creates a
fire-and-forget task (feed_update_task) and only uses
_background_feed_update_tasks.discard as the done callback, which hides task
exceptions; change the done callback to a small wrapper that 1) removes the task
from _background_feed_update_tasks and 2) checks task.exception() and surfaces
it (e.g., call self.logger.exception(...) or pass to the event loop's exception
handler) so any errors in _background_feed_update are logged/observed with
context instead of being silently dropped.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7ae632d5-062b-45d7-9373-7b5f950e7a45

📥 Commits

Reviewing files that changed from the base of the PR and between dc83170 and ad90c33.

📒 Files selected for processing (3)

• src/aiogram_webhook/engines/base.py
• src/aiogram_webhook/engines/simple.py
• src/aiogram_webhook/engines/token.py

🚧 Files skipped from review as they are similar to previous changes (1)

• src/aiogram_webhook/engines/simple.py

[coderabbitai]

♻️ Duplicate comments (1)

src/aiogram_webhook/engines/token.py (1)

59-60: ⚠️ Potential issue | 🟠 Major

Catch malformed token parsing in _get_bot_by_token and align with nullable contract.

Line 60 calls extract_bot_id(token) without handling parse failures. Malformed tokens can raise and bubble as 500s in request flow, while src/aiogram_webhook/engines/base.py (Line 78) defines a nullable return path (Bot | None) for this method.

Suggested fix

-    async def _get_bot_by_token(self, token: str) -> Bot:
-        bot_id = extract_bot_id(token)
+    async def _get_bot_by_token(self, token: str) -> Bot | None:
+        try:
+            bot_id = extract_bot_id(token)
+        except ValueError:
+            return None
         existing_bot = self._bots.get(bot_id)

         if existing_bot is None or existing_bot.token != token:
             new_bot = self._build_bot(token)
             self._bots[bot_id] = new_bot
             return new_bot

         return existing_bot

-        bot = await self._get_bot_by_token(token=token)
+        bot = await self._get_bot_by_token(token=token)
+        if bot is None:
+            raise ValueError("Malformed bot token")

#!/bin/bash
set -euo pipefail

# Verify current implementation vs base contract and error handling.
rg -nP --type=py -C3 'async def _get_bot_by_token|extract_bot_id\(|except ValueError' src/aiogram_webhook/engines/token.py
rg -nP --type=py -C2 'async def _get_bot_by_token\(self, token: str\) -> Bot \| None' src/aiogram_webhook/engines/base.py

Also applies to: 96-96

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/aiogram_webhook/engines/token.py` around lines 59 - 60, The call to
extract_bot_id(token) inside _get_bot_by_token can raise on malformed tokens and
must not bubble as a 500; wrap the extract_bot_id(token) invocation in a
try/except (catch ValueError or the specific parse error) and return None when
parsing fails so the method adheres to the nullable return contract (Bot | None)
declared in the base class; apply the same defensive parsing pattern to the
other extract_bot_id usage around the second occurrence (near the other call you
noted) so both code paths return None on parse failure instead of raising.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@src/aiogram_webhook/engines/token.py`:
- Around line 59-60: The call to extract_bot_id(token) inside _get_bot_by_token
can raise on malformed tokens and must not bubble as a 500; wrap the
extract_bot_id(token) invocation in a try/except (catch ValueError or the
specific parse error) and return None when parsing fails so the method adheres
to the nullable return contract (Bot | None) declared in the base class; apply
the same defensive parsing pattern to the other extract_bot_id usage around the
second occurrence (near the other call you noted) so both code paths return None
on parse failure instead of raising.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 97b661ad-b44c-4455-be3a-e9014e459ecc

📥 Commits

Reviewing files that changed from the base of the PR and between f2431c0 and 8a5302c.

📒 Files selected for processing (9)

• src/aiogram_webhook/engines/base.py
• src/aiogram_webhook/engines/simple.py
• src/aiogram_webhook/engines/token.py
• src/aiogram_webhook/security/checks/check.py
• src/aiogram_webhook/security/checks/ip.py
• src/aiogram_webhook/security/secret_token.py
• src/aiogram_webhook/security/security.py
• tests/fixtures/fixtures_checks.py
• tests/test_secret_token.py

[coderabbitai]

♻️ Duplicate comments (1)

README.md (1)

289-289: ⚠️ Potential issue | 🟡 Minor

Clarify that routing methods must be async.

The documentation does not explicitly state that webhook_url() and resolve_token() must be implemented as async methods. Users implementing custom routing may incorrectly use regular def instead of async def, leading to runtime errors or incorrect behavior.

📝 Suggested clarification

-You can implement your own routing by inheriting from `BaseRouting` or `TokenRouting` and implementing the `webhook_url()` method (and `resolve_token()` if using token-based routing).
+You can implement your own routing by inheriting from `BaseRouting` or `TokenRouting` and implementing async `webhook_url()` method (and async `resolve_token()` if using token-based routing).

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@README.md` at line 289, Update the README sentence to explicitly state that
implementations of BaseRouting.webhook_url and TokenRouting.resolve_token must
be asynchronous (use async def); mention the required async signatures for
webhook_url() and, when applicable, resolve_token() and include a short note
that they should return the expected types (e.g., webhook_url -> str) so
implementers use async def webhook_url(...) and async def resolve_token(...)
rather than regular def.

🧹 Nitpick comments (1)

README.md (1)

203-210: Avoid documenting private methods in user-facing documentation.

The request processing section references private methods (_get_bot_token_for_request, _get_bot_by_token) that are prefixed with underscores, indicating they are internal implementation details. User-facing documentation should focus on the conceptual flow and public API rather than exposing private method names, which may confuse users or encourage misuse of the internal API.

📝 Suggested revision focusing on conceptual flow

 ### Request processing
 
 `WebhookEngine` handles incoming updates in this order:
 
-1. Extract token from request (`_get_bot_token_for_request`)
-2. Run security checks for the token (`Security.verify(token, bound_request)`)
-3. Resolve bot (`_get_bot_by_token`)
+1. Extract token from request
+2. Run security checks for the token (via `Security.verify()`)
+3. Resolve bot instance from token
 4. Pass update to aiogram dispatcher

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@README.md` around lines 203 - 210, The README currently lists private method
names (_get_bot_token_for_request, _get_bot_by_token) in the Request processing
section; replace those concrete private symbols with conceptual, user-facing
descriptions (e.g., "extract token from the incoming request" and "resolve the
bot associated with the token") so the flow describes behavior rather than
internal method names, and ensure only public-facing components (like
WebhookEngine and Security.verify as a security step) are mentioned.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@README.md`:
- Line 289: Update the README sentence to explicitly state that implementations
of BaseRouting.webhook_url and TokenRouting.resolve_token must be asynchronous
(use async def); mention the required async signatures for webhook_url() and,
when applicable, resolve_token() and include a short note that they should
return the expected types (e.g., webhook_url -> str) so implementers use async
def webhook_url(...) and async def resolve_token(...) rather than regular def.

---

Nitpick comments:
In `@README.md`:
- Around line 203-210: The README currently lists private method names
(_get_bot_token_for_request, _get_bot_by_token) in the Request processing
section; replace those concrete private symbols with conceptual, user-facing
descriptions (e.g., "extract token from the incoming request" and "resolve the
bot associated with the token") so the flow describes behavior rather than
internal method names, and ensure only public-facing components (like
WebhookEngine and Security.verify as a security step) are mentioned.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 65352360-06ee-4e69-b186-23e5efaadcb9

📥 Commits

Reviewing files that changed from the base of the PR and between 8a5302c and 0fad9c3.

📒 Files selected for processing (1)

• README.md

[Copilot]

Pull request overview

This PR refactors webhook routing and security to operate primarily on a bot token string (instead of requiring an aiogram.Bot instance up front), and updates engines/tests/docs accordingly.

Changes:

• Refactor WebhookEngine.handle_request flow to: extract token → run security checks → resolve bot → dispatch update.
• Make routing and token-resolution APIs async (webhook_url(), resolve_token()), and update engines/tests to await them.
• Rework secret-token verification to use a shared header constant and an async SecretToken.secret_token() provider API.

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/aiogram_webhook/engines/base.py	Updates request handling pipeline (token-first) and registration path (webhook_path); adds a warning when security is unset.
src/aiogram_webhook/engines/token.py	Refactors token engine bot caching/resolution; introduces shared session creation; updates webhook setup to new routing/security APIs; adds bots view + remove_bot.
src/aiogram_webhook/engines/simple.py	Adapts single-bot engine to new _get_bot_token_for_request / _get_bot_by_token abstractions and async routing/security API.
src/aiogram_webhook/routing/base.py	Introduces webhook_path property and async webhook_url() / resolve_token() abstract APIs.
src/aiogram_webhook/routing/static.py	Renames/changes static routing to async webhook_url().
src/aiogram_webhook/routing/path.py	Converts path routing to async webhook_url() and async token resolution.
src/aiogram_webhook/routing/query.py	Converts query routing to async webhook_url() and async token resolution.
src/aiogram_webhook/security/security.py	Changes security checks to accept bot_token instead of Bot; renames secret-token accessor to secret_token().
src/aiogram_webhook/security/secret_token.py	Introduces SECRET_TOKEN_HEADER; makes SecretToken.verify() concrete and secret_token() async/abstract; updates StaticSecretToken.
src/aiogram_webhook/security/checks/check.py	Updates SecurityCheck protocol to accept bot_token: str.
src/aiogram_webhook/security/checks/ip.py	Updates IP check signature to accept bot_token: str.
src/aiogram_webhook/config/bot.py	Uses @dataclass(slots=True) for BotConfig.
tests/test_security.py	Updates tests to pass bot_token and use SECRET_TOKEN_HEADER.
tests/test_secret_token.py	Updates tests for new Security.secret_token() API and header constant.
tests/test_ip_check.py	Updates IP check tests for new bot_token-based API.
tests/test_routing.py	Converts routing tests to async and awaits new routing methods.
tests/fixtures/fixtures_checks.py	Updates fixture security checks to accept bot_token.
README.md	Updates documentation for renamed engine/routing APIs and new request-processing order.
.github/workflows/tests.yml	Adds ty check to CI workflow.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.