Skip to content

fix(yuanbao): clear RecallGuard tracking per turn + TTL-evict member cache (salvage #23383 #23384) - #77606

Merged
kshitijk4poor merged 3 commits into
NousResearch:mainfrom
kshitijk4poor:salvage/23383-23384-yuanbao-state
Aug 3, 2026
Merged

fix(yuanbao): clear RecallGuard tracking per turn + TTL-evict member cache (salvage #23383 #23384)#77606
kshitijk4poor merged 3 commits into
NousResearch:mainfrom
kshitijk4poor:salvage/23383-23384-yuanbao-state

Conversation

@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Salvage of the yuanbao pair #23383 + #23384 by @AhmetArif0 — both commits cherry-picked verbatim (authorship preserved) + one review follow-up commit. One coherent PR since both fix per-adapter state lifetimes in the same file.

What this does for users

Two unbounded-state leaks in the Yuanbao adapter, both real on current main:

  1. RecallGuard tracking never cleaned (fix(yuanbao): clear _processing_msg_ids/_processing_msg_texts after each message #23383): _dispatch_inbound_event writes _processing_msg_ids[session_key] / _processing_msg_texts[session_key] for every id-bearing message, and nothing ever removes them — the wrapper _process_message_background had no cleanup. Long-running gateways accumulate one entry per active session forever, and stale entries can match a recalled msg_id from a long-finished turn. Now cleared in the wrapper's finally.
  2. Expired member-cache entries never evicted (fix(yuanbao): evict stale entries from _member_cache on TTL expiry #23384): _build_msg_body_with_mentions treated an expired _member_cache entry as empty but left it stored — the dict only shrinks if the same group later refetches. Expired entries are now deleted at read time (no await between get and del; single-threaded event loop makes this safe).

Review follow-up (the sweeper's concern, verified and fixed)

The original #23383 guard popped on not msg_id or <match>. The not msg_id arm is wrong: id-less events (internal/synthetic) never WROTE a tracking entry, so any entry they observe belongs to a concurrently-queued id-bearing message whose drain task still needs it for recall matching. The base adapter explicitly spawns that pending event as a drain task during final cleanup, so the erase is reachable. Fixed to pop only on truthy + matching msg_id.

Verification

  • 5 new regression tests (tests/gateway/platforms/test_yuanbao_state_cleanup.py): normal cleanup, id-less non-erasure, overwritten-entry ownership handoff, TTL eviction, fresh-entry survival — plus the 3 existing yuanbao suites green
  • MUTATION-CHECKED both ways: full revert of yuanbao.py to base → cleanup + eviction tests fail; re-introducing the original not msg_id guard → exactly the id-less non-erasure test fails (the follow-up's guard binds)

Closes #23383. Closes #23384.

AhmetArif0 and others added 3 commits August 3, 2026 16:58
…ach message

_dispatch_inbound_event() writes session_key → msg_id/raw_text into
_processing_msg_ids and _processing_msg_texts so RecallGuardMiddleware
can find and interrupt the currently-processing message.  These entries
were never removed after a message finished processing, causing both
dicts to grow unboundedly — one persistent entry per unique session key
for the lifetime of the bot.

Fix: clear both entries in the _process_message_background() finally
block, after super() returns.  The guard compares the stored msg_id
against event.message_id before popping: a concurrent pending message
may have already overwritten the entry in _dispatch_inbound_event while
we were running, in which case the drain task owns it and we must not
clear it.  When msg_id is absent (nothing was written at dispatch time)
the pop is a safe no-op.

Note: _msg_content_cache already bounds itself to 200 entries at the
same write site; _processing_msg_ids and _processing_msg_texts had no
such bound.
_build_msg_body_with_mentions() checks the TTL of each _member_cache
entry and returns an empty member list when the entry is stale, but
never removes the entry from the dict.  Over time every group_code the
bot has ever queried accumulates a permanent entry, retaining the full
member list (potentially thousands of records per group) until
disconnect().

Fix: delete the stale entry at the point it is detected as expired.
The next call to get_group_member_list_raw() for the same group will
repopulate the cache with fresh data as before.

Symmetric with the existing TTL pattern in MessageDeduplicator, which
evicts on access.
…regression tests

Follow-up on the salvaged pair: the original guard's `not msg_id` arm let an
id-less internal/synthetic event erase a tracking entry a concurrently-queued
id-bearing message's drain task still needs for recall matching (id-less
events never write entries in _dispatch_inbound_event, so they must never
pop). Tests cover: normal cleanup, id-less non-erasure, overwritten-entry
ownership handoff, TTL eviction + fresh-entry survival.
@kshitijk4poor
kshitijk4poor enabled auto-merge (rebase) August 3, 2026 11:38
@kshitijk4poor
kshitijk4poor disabled auto-merge August 3, 2026 11:38
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Aug 3, 2026
@kshitijk4poor
kshitijk4poor enabled auto-merge (rebase) August 3, 2026 12:03
@kshitijk4poor
kshitijk4poor merged commit f3add02 into NousResearch:main Aug 3, 2026
42 checks passed
@kshitijk4poor
kshitijk4poor deleted the salvage/23383-23384-yuanbao-state branch August 5, 2026 07:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants