fix(acp): re-subscribe channels the relay layer has dropped - #5014
Open
joe-rodgers wants to merge 1 commit into
Open
fix(acp): re-subscribe channels the relay layer has dropped#5014joe-rodgers wants to merge 1 commit into
joe-rodgers wants to merge 1 commit into
Conversation
`subscribed_channel_ids` in the harness main loop records the intent to enqueue a subscribe command, not a confirmed subscription: `subscribe_channel` / `subscribe_channel_from` only queue a REQ on the background task's command channel and return Ok. The relay layer can then permanently drop a channel behind the harness's back — `drop_channel_on_access_denied` (member evicted / open→private flip) and the two "missing filter — dropping" paths in `drain_rate_limited_pending` / `drain_resubscribe_retry`. All three clear `active_subscriptions` / `active_filters`, so reconnect never restores the channel either. Nothing told lib.rs. The `if subscribed_channel_ids.contains(&ch)` guard on the member-added path then made every subsequent membership notification for that channel a silent no-op, and the channel stayed dead until the process restarted, with no log line saying so. Fix: the background task records each permanent drop into a `DroppedChannels` set shared with `HarnessRelay`. The main loop drains it via `take_dropped_channels()` before the "already subscribed" guard runs and clears those ids, so the next member-added notification re-subscribes instead of short-circuiting. A fresh Subscribe command clears the drop mark so a rebuilt subscription is not torn down again. Every drop now logs at warn. Tests: relay-side (drop is reported, connection-level denials are not, re-subscribe clears the mark, missing-filter paths report) and harness-side (a dropped channel takes the subscribe path on the next membership notification; unrelated channels untouched; repeatable). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015KzSArAnn5xntFSNurJDH3
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
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
subscribed_channel_idsin the harness main loop records the intent to enqueue a subscribe command, not a confirmed subscription:subscribe_channel/subscribe_channel_fromonly queue a REQ on the background task's command channel and returnOk.The relay layer can then permanently drop a channel behind the harness's back —
drop_channel_on_access_denied(member evicted / open→private flip) and the two "missing filter — dropping" paths indrain_rate_limited_pending/drain_resubscribe_retry. All three clearactive_subscriptions/active_filters, so reconnect never restores the channel either.Nothing told
lib.rs. Theif subscribed_channel_ids.contains(&ch)guard on the member-added path then made every subsequent membership notification for that channel a silent no-op, and the channel stayed dead until the process restarted — with no log line saying so.Fix
The background task records each permanent drop into a
DroppedChannelsset shared withHarnessRelay. The main loop drains it viatake_dropped_channels()before the "already subscribed" guard runs and clears those ids, so the next member-added notification re-subscribes instead of short-circuiting. A freshSubscribecommand clears the drop mark so a rebuilt subscription is not torn down again. Every drop now logs atwarn.Testing
Relay-side: drop is reported, connection-level denials are not, re-subscribe clears the mark, missing-filter paths report. Harness-side: a dropped channel takes the subscribe path on the next membership notification, unrelated channels untouched, repeatable.
cargo test -p buzz-acp --release: 675 passed, 0 failed.cargo fmt --checkclean.Context
Found on a long-running self-hosted fleet where individual agents would go quiet in one channel while still answering in others, with nothing in the logs to distinguish it from an idle agent.