Skip to content

feat(buzz): reconcile joined channels live from the membership roster - #112813

Open
cmyk wants to merge 1 commit into
NousResearch:mainfrom
Peakhunter:feat/buzz-channel-membership
Open

cmyk wants to merge 1 commit into
NousResearch:mainfrom
Peakhunter:feat/buzz-channel-membership

Conversation

@cmyk

@cmyk cmyk commented Sep 16, 2026

Copy link
Copy Markdown

Keeps the Buzz adapter's watched channels in step with community membership while the Gateway runs.

What changes

  • The watch set is the authoritative joined roster (buzz channels list --member), validated row by row before it is applied. channels / BUZZ_CHANNELS can only narrow that set, never widen it. If neither a joined channel nor a DM exists, connect fails with config_missing as before.
  • Membership events of kind 44100 and 44101 re-read the roster: newly joined channels are seeded and subscribed, departed channels have their subscription closed and their state dropped. A targeted event for a channel that is neither joined nor watched is ignored, and a future-dated created_at cannot move the reconnect cursor past "now".
  • The poll transport reconciles the roster on the same cadence and a malformed or failed roster read keeps the previous snapshot.
  • DM discovery is unchanged and stays independent of the group roster.
  • Lifecycle: when the identity lock is already held, no lock key is left behind for disconnect() to release, and every failed connect path tears down the partial transport.

Verification

scripts/run_tests.sh on tests/gateway/test_buzz_channel_membership.py, test_buzz_identity_lifecycle.py, test_buzz_adapter.py and test_buzz_websocket.py: 227 tests pass.

This is the live channel reconciliation and lock-lifecycle portion of #90802, rebased on current main and taken from the code running in production on our Hermes fork. #90802 is closed; its handoff behaviour already lives in #77647, and its thread-root routing will follow as its own PR.

@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference; please use your judgment.

The PR reconciles the Buzz watch set live against the authoritative joined roster (channels list --member replaces plain channels list in connect, DM discovery, and the new _discover_joined_channels), adds kind-44101 removal events over WebSocket, restructures connect() so every post-acquire failure path releases the identity lock via a finally (and a failed acquire no longer leaves the holder's key on the adapter), and narrows BUZZ_CHANNELS/channels to a filter that cannot widen the joined set. The lock lifecycle fix and the untrusted-event guard (reconciliation proceeds only when the event's h target is in the CLI roster or already watched, and a failed roster fetch changes nothing) are sound, and the tests pin both sides of the add/remove flow; the find below is a concurrency defect in the new subscription path.

  1. subscribe_discovered iterates _channel_state live across an await while a sibling task mutates it — non-blocking
    • plugins/platforms/buzz/adapter.py:1110-1125 — for channel_id in self._channel_state: with await self._send_channel_subscription(...) inside the loop body (1119-1123). On the WebSocket transport this runs on the _ws_read_loop task while the separate _ws_discovery_loop task concurrently runs _rediscover_and_subscribe_discover_dms, which inserts and removes _channel_state keys. A mutation landing between the loop's await points raises RuntimeError: dictionary changed size during iteration; _ws_read_loop has no per-frame exception handler, so it propagates to _websocket_loop's except Exception and tears the socket down for a backoff reconnect — triggered by an ordinary membership event coinciding with a discovery sweep.
    • The pre-existing _rediscover_and_subscribe iterates a list(self._channel_state) snapshot (the idiom this new helper dropped); the CLOSE loop in the same method already snapshots correctly (list(subscriptions.items())).
    • Suggested fix: iterate list(self._channel_state) in subscribe_discovered (both call sites, before and after _discover_dms).

Minor: the _poll_loop elif is dead code — adapter.py:1283-1284 tests _poll_count % _DM_DISCOVERY_EVERY only when _poll_count % _CHANNEL_DISCOVERY_EVERY != 0, but both constants are 5 and the taken branch already runs _discover_dms, so the elif can never fire. Either drop it or give the roster sweep its own cadence if a slower/faster split was intended.

Minor: connect() now hard-requires channels list --member (adapter.py:662; likewise 1299 and 1459). The current block/buzz CLI ships the flag as a bare boolean, but an older installed binary exits with a clap usage error on the unknown flag, which connect() maps to retryable (exit code 2) — a perpetual retry loop whose only hint is the CLI's stderr. Consider falling back to plain channels list for that specific error, or surfacing a targeted "buzz CLI too old for --member" message.

@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/plugins Plugin system and bundled plugins labels Sep 17, 2026
Watch only the authoritative joined roster (channels list --member) and
validate every row before applying it. Membership events (kinds 44100 and
44101) and the poll cadence re-read the roster, subscribing to newly joined
channels and closing subscriptions for departed ones without a Gateway
restart; a malformed roster keeps the previous snapshot and an explicit
channels list can only narrow the joined set. A failed identity-lock
acquisition no longer leaves a lock key behind for disconnect to release,
and any failed connect path tears the partial transport down.
@cmyk

cmyk commented Sep 17, 2026

Copy link
Copy Markdown
Author

Thanks, all three addressed; the branch is updated in place (single commit, force-pushed).

  • Live iteration across an await. subscribe_discovered now iterates a list(self._channel_state) snapshot at both call sites, matching the idiom the rest of the file already uses.
  • Dead elif. _CHANNEL_DISCOVERY_EVERY is gone; the roster sweep runs in the existing _DM_DISCOVERY_EVERY branch before DM discovery. Same cadence as before, one fewer constant.
  • Old CLI without --member. connect() recognises the argument parser's rejection of the flag and fails non-retryable with buzz CLI does not support 'channels list --member'; upgrade the buzz binary instead of looping on exit code 2. A plain channels list fallback is deliberately not offered: that listing is the public catalogue, not the joined roster, and silently widening the watch set on an old binary is the failure this PR removes. New test covers it.

228 tests across the Buzz adapter, websocket, membership and lifecycle files pass on the updated commit.

@reinhold-ph
reinhold-ph force-pushed the feat/buzz-channel-membership branch from 9fa794c to ffebd7b Compare September 17, 2026 05:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants