Skip to content

/reset can race an in-flight turn and session creation #168

Description

@QueryPlanner

What

Found during a code-reduction audit of the Telegram /reset command and ADK session creation. Two related concurrency gaps.

  1. /reset bypasses per-conversation task serialization. Every normal message and album turn goes through _run_sequenced_turn (bot.py), which tracks _conversation_tasks/_conversation_task_seqs to cancel/wait-out any superseded turn for the same conversation before starting a new one. _handle_reset (bot.py:945) is dispatched directly from _handle_command and never goes through that sequencing — so a /reset can run concurrently with an in-flight message-handling task for the same conversation_key, with no cancellation or ordering guarantee between them.
  2. AdkRuntime.create_next_session has no lock around its read-then-write. create_next_session (adk_runtime.py:340) reads the existing session via _get_latest_session (adk_runtime.py:618) then computes next_version = current_version + 1 with no mutex. Combined with bug 1, two concurrent resets (or a reset racing an in-flight turn that also calls get_or_create_session/create_next_session) can both read the same current_version and create colliding or duplicate versioned sessions.
  3. Related, smaller inconsistency: _handle_reset calls _build_session_state without a chat_type argument, unlike the other four call sites (_handle_message, _handle_photo_upload, _handle_file_upload, _handle_album_turn), which all pass chat_type=chat_type or self._chat_type_context.get(). A session created via /reset is missing telegram_chat_type in its state while sessions created via normal messages have it — an inconsistent state schema depending on which path created the session.

Why it matters

This is a real (if narrow-window) race: a user sending /reset right after (or during) an in-flight message could end up with duplicate/colliding session versions, or a reset that gets silently superseded/ignored by a concurrent turn. It's also the underlying reason /reset is currently the only reliable way to recover from the day-rollover session-continuity issue discussed earlier in this conversation — any future fix that makes session rollover automatic (rather than manual /reset) will hit this same race more often.

Priority

Medium — narrow timing window today (a user has to send /reset at almost the same instant as another message), but it's a real correctness gap in session identity, and any future auto-reset-on-new-day feature would make it much easier to trigger.

Level of Effort

Medium (M) — routing /reset through _run_sequenced_turn is straightforward given it already exists; adding a lock around create_next_session's read-then-write (or making session-version allocation atomic at the storage layer) is a bit more involved and touches adk_runtime.py, a more central/shared module. The chat_type fix is trivial (pass the same argument the other four call sites already do).

Sources

Passing criteria / definition of done

  • _handle_reset is routed through the same conversation-task sequencing (_run_sequenced_turn or equivalent) as other turns; a test asserts a /reset cancels/waits-out an in-flight turn for the same conversation the same way a new message would.
  • A test (or a documented architectural decision, if a lock is judged unnecessary) covers two concurrent create_next_session calls for the same SessionLocator and asserts they do not produce two sessions with the same version number.
  • _handle_reset passes chat_type to _build_session_state consistently with the other four call sites; a test asserts the resulting state includes telegram_chat_type when available.
  • pytest tests/test_telegram_bot.py -q and the adk_runtime session tests pass with the new assertions.
  • ruff check, ruff format --check, and mypy src/blacki/ all pass with no new warnings.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions