Repository navigation
fix(proactive-routing): handle non-bridge channels (voice / github-commits) — closes residual half of #988 - #1031
Merged
sonichi merged 1 commit intoMay 22, 2026
Conversation
…mmits) — closes residual half of sonichi#988 ## The bug — surfaced by @rickchen007 on fork PR sonichi#35 sonichi#988 (this PR's antecedent on upstream) introduced `should_claim_proactive` to route `proactive-*.txt` files to the bridge whose channel the owner was last active on. But `state/last-owner-activity.json` is written by MORE producers than just the discord-bridge and telegram- bridge. Concrete values that real installs write: - `{"channel": "voice", ...}` — the voice agent writes on every owner utterance - `{"channel": "github-commits", ...}` — the github auto-poll writes on every observed new commit These are activity-tracking signals, not message-delivery channels. Pre-fix sonichi#988 used strict `last_channel == this_channel`. For `{"channel": "voice"}`: should_claim_proactive(state, "discord") -> False should_claim_proactive(state, "telegram") -> False → **No bridge claims the proactive-*.txt file.** The owner notification is silently stranded in `results/` until the owner next sends a Discord/Telegram message that restores a recognized channel. Voice is a VERY common last-active surface — owner using voice agent writes voice activity, then a downstream task generates a proactive, and all bridges skip → silence. Real correctness bug; thanks @rickchen007 for catching it. ## Fix `BRIDGE_CHANNELS = frozenset({"discord", "telegram"})` — the set of channels whose bridges actually compete for proactive files. The routing rule: - `last_channel ∈ BRIDGE_CHANNELS` → strict match (`last_channel == this_channel`). - Else (non-bridge value, missing, malformed, empty, non-dict) → default Discord. Discord-as-default preserves the existing "new install → Discord" behavior AND closes the strand class. ## Tests Updated `tests/proactive-routing.test.py`: - REMOVED `test_unknown_channel_value_does_not_match_anyone` (asserted the buggy "both False" behavior). - ADDED `test_voice_channel_defaults_to_discord` — pins the concrete bug. - ADDED `test_github_commits_channel_defaults_to_discord` — same shape for the github-poll producer. - ADDED `test_unrecognized_channel_defaults_to_discord` — general rule (any non-bridge channel). - ADDED `test_bridge_channels_set_is_documented` — pins the `BRIDGE_CHANNELS` constant so a future contributor adding a new bridge must update the set AND add a routing test deliberately. 11 tests total, all passing locally. Co-Authored-By: rickchen007 (review feedback on fork PR sonichi#35) Co-Authored-By: Vasiliy's Sutando <noreply@anthropic.com>
liususan091219
pushed a commit
to liususan091219/sutando-fork
that referenced
this pull request
May 28, 2026
…mmits) — closes residual half of sonichi#988 (sonichi#1031) ## The bug — surfaced by @rickchen007 on fork PR sonichi#35 sonichi#988 (this PR's antecedent on upstream) introduced `should_claim_proactive` to route `proactive-*.txt` files to the bridge whose channel the owner was last active on. But `state/last-owner-activity.json` is written by MORE producers than just the discord-bridge and telegram- bridge. Concrete values that real installs write: - `{"channel": "voice", ...}` — the voice agent writes on every owner utterance - `{"channel": "github-commits", ...}` — the github auto-poll writes on every observed new commit These are activity-tracking signals, not message-delivery channels. Pre-fix sonichi#988 used strict `last_channel == this_channel`. For `{"channel": "voice"}`: should_claim_proactive(state, "discord") -> False should_claim_proactive(state, "telegram") -> False → **No bridge claims the proactive-*.txt file.** The owner notification is silently stranded in `results/` until the owner next sends a Discord/Telegram message that restores a recognized channel. Voice is a VERY common last-active surface — owner using voice agent writes voice activity, then a downstream task generates a proactive, and all bridges skip → silence. Real correctness bug; thanks @rickchen007 for catching it. ## Fix `BRIDGE_CHANNELS = frozenset({"discord", "telegram"})` — the set of channels whose bridges actually compete for proactive files. The routing rule: - `last_channel ∈ BRIDGE_CHANNELS` → strict match (`last_channel == this_channel`). - Else (non-bridge value, missing, malformed, empty, non-dict) → default Discord. Discord-as-default preserves the existing "new install → Discord" behavior AND closes the strand class. ## Tests Updated `tests/proactive-routing.test.py`: - REMOVED `test_unknown_channel_value_does_not_match_anyone` (asserted the buggy "both False" behavior). - ADDED `test_voice_channel_defaults_to_discord` — pins the concrete bug. - ADDED `test_github_commits_channel_defaults_to_discord` — same shape for the github-poll producer. - ADDED `test_unrecognized_channel_defaults_to_discord` — general rule (any non-bridge channel). - ADDED `test_bridge_channels_set_is_documented` — pins the `BRIDGE_CHANNELS` constant so a future contributor adding a new bridge must update the set AND add a routing test deliberately. 11 tests total, all passing locally. Co-Authored-By: rickchen007 (review feedback on fork PR sonichi#35) Co-authored-by: Vasiliy's Sutando <noreply@anthropic.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
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.
The bug — surfaced by @rickchen007 on fork PR #35
PR #988 introduced
should_claim_proactiveto routeproactive-*.txtfiles to the bridge whose channel the owner was last active on. Butstate/last-owner-activity.jsonis written by more producers than just the discord-bridge and telegram-bridge. Concrete values real installs write:{\"channel\": \"voice\", ...}— voice agent on every owner utterance{\"channel\": \"github-commits\", ...}— github auto-poll on every observed new commitThese are activity-tracking signals, not message-delivery channels.
Pre-fix #988 used strict
last_channel == this_channel. For{\"channel\": \"voice\"}:```
should_claim_proactive(state, "discord") -> False
should_claim_proactive(state, "telegram") -> False
```
→ No bridge claims the
proactive-*.txtfile. The owner notification is silently stranded inresults/until the owner next sends a Discord/Telegram message that restores a recognized channel.Voice is a very common last-active surface — owner using voice agent writes voice activity, then a downstream task generates a proactive, all bridges skip → silence.
Fix
BRIDGE_CHANNELS = frozenset({\"discord\", \"telegram\"})— the set of channels whose bridges actually compete for proactive files. Routing rule:last_channel ∈ BRIDGE_CHANNELSlast_channel == this_channel)voice/github-commits/ missing / malformed)Discord-as-default preserves the existing "new install → Discord" behavior AND closes the strand class.
Tests
11 tests in
tests/proactive-routing.test.py:test_voice_channel_defaults_to_discord(the concrete bug)test_github_commits_channel_defaults_to_discordtest_unrecognized_channel_defaults_to_discord(general rule)test_bridge_channels_set_is_documented(pins the constant so future contributors must update test AND set together)All 11 pass locally.
Acknowledgements
@rickchen007 caught this on fork PR #35. Same fix landing here closes the residual half of #988 upstream.
🤖 Generated with Claude Code