fix(acp): periodically reconcile channel membership - #6661
Open
jaxxon33 wants to merge 2 commits into
Open
Conversation
The live kind:44100 member-added notification is the only trigger that makes an already-running harness subscribe to a newly joined channel, and channel discovery otherwise runs exactly once at startup. Roster mutations that bypass relay side effects (operator consoles writing the store directly) or that happen while the harness is offline therefore leave running agents deaf to those channels until restart. Add an additive-only periodic sweep (default 90s, configurable via --channel-discovery-refresh-secs / BUZZ_ACP_CHANNEL_DISCOVERY_REFRESH_SECS, 0 disables) that re-runs discovery and subscribes to anything new using the same rule resolution as the live member-added path. Unsubscription stays owned by the kind:44101 path so a transient partial discovery can never evict live subscriptions. Signed-off-by: Paul <paul@x.net.au>
Signed-off-by: Paul <paul@x.net.au>
jaxxon33
changed the title
Chessing234
suggested changes
Aug 25, 2026
Chessing234
left a comment
Chessing234
left a comment
Contributor
There was a problem hiding this comment.
additions apply on the first sweep, so a leave that hasn't landed on the relay gets undone 90s later. removals already need two hits — adds should too, or skip ids with a local leave in flight.
loganj
added a commit
that referenced
this pull request
Sep 8, 2026
🤖 ## Summary When a Buzz agent falls behind on incoming messages, its connection can make the backlog worse while trying to recover. The connection buffers messages from the relay server until the agent is ready to process them; if that buffer overflows, recovery previously requested history for **every subscribed channel** and paused socket reads while sending those requests. That adds traffic to an already overloaded connection. This change requests history only for affected subscriptions, once the code consuming those messages has room, with at least five seconds between attempts. The recovery path now: - Combines repeated losses into one pending recovery per affected subscription, keeping the oldest dropped timestamp so replay starts early enough. - Waits until at least half the consumer queue is free and the relay's existing rate-limit delay has expired. The queue wakes recovery when space becomes available; recovery does not periodically sample capacity or hold queue space away from live messages. - Attempts one subscription at a time, choosing the least recently attempted so a busy channel cannot crowd out other channels or membership notifications. The five-second delay starts when an attempt finishes, including a failed write; failed writes leave recovery pending. Recovery is paced by available capacity, not by how often messages are lost. This is not a larger buffer or a cutoff that abandons recovery. Subscription identifiers, message filters, replay timestamp overlap and duplicate filtering are unchanged; no downstream agent changes are required. This targets a reproducible overload **amplifier**, not every cause of overload or every catch-up limitation. The initial live overload's cause has not been established. Recovery remains best effort: a successful request write is not proof of delivery, and existing history/retention limits, bounded duplicate tracking and replay limitations still apply. There is no exactly-once or complete catch-up guarantee. A stalled write can still pause socket reads for the existing ten-second timeout; the pacing bound does not cover initial subscriptions, reconnects or other retry paths. ### Related issue Closest related: #5014 (channel re-subscription); also #6661 (membership reconciliation) and #6090 (relay backpressure gap signaling). This addresses local overflow recovery scheduling, not those separate mechanisms. ### Testing Recorded offline comparisons against the previous behavior, with the final implementation at `8000636f3073167c5a5107bb179c7d91160f1729`: | Same fixture: 18 subscriptions, three overload rounds | Before | After | | --- | --- | --- | | Recovery history requests | 108 | 3 | | Ping-response delay | About 4.6 seconds | Below the measurement's 1 ms resolution | A separate bounded-history fixture delivered all 320 events plus subsequent live traffic in **both** versions. Regression coverage exercises the real socket-handling task, including intermittent consumer capacity, fairness, failed writes and cancellation of capacity waits before live delivery. These are synthetic results, not production throughput measurements or evidence of a deployed cure. The full local `RUST_TEST_THREADS=4 just ci` run passed on September 4, 2026. Earlier unsuccessful local runs remain part of the validation history. The [recorded validation evidence and separate desktop follow-up](#7325 (comment)) preserve the original desktop mock-history scroll failure, its passing rerun and the remaining investigation. That desktop path does not run the agent connection code; neither this repair nor the passing rerun fixes the observed scroll problem. --------- Signed-off-by: Logan Johnson <loganj@squareup.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A running
buzz-acpharness primarily learns about channel membership from startup discovery and live kind:44100/44101 notifications. Notifications can still be missed while the harness is offline, during transient backpressure/outages, or when interoperating with older writers, leaving an agent subscribed to the wrong channel set until restart.Fix
Add a periodic safety reconciliation with these safeguards:
channel_discovery_refresh_secs(default 90s;0disables it).The web console's separate direct-writer issue is being fixed independently by publishing owner-signed membership events through the relay; this PR remains the defense-in-depth recovery path.
Testing
At commit
f453c56b333eb4de421d676a3c05f399f0ea62d7:cargo fmt --all -- --check- passcargo clippy -p buzz-acp --all-targets -- -D warnings- passcargo test -p buzz-acp --lib- 805 passed, 0 failed./bin/just ciwas attempted but the host lackspkg-config/OpenSSL development tooling, so its workspace clippy phase cannot start. The affected package's full gates above are clean.Commits are signed off per DCO.