fix(daemon): @sam in non-broadcast member channels misclassified as side-pane#85
Merged
Merged
Conversation
…nnel Bug confirmed in production logs 2026-05-25 on @-mention in #team-engineering: 19:20:07 ignoring event from non-whitelisted channel C0884QPQF6W 19:20:29 redirected side-pane message in C0884QPQF6W Slack staples an `assistant_thread` field onto events for assistant-type apps even when the event arrives on a real workspace channel. The previous short-circuit only excluded SAM_CHANNEL (or `broadcast_channel_id` after the aed41ba refactor) — leaving every OTHER member channel vulnerable: on_app_mention correctly ignored (SAM_CHANNEL narrowed scope), but on_message saw the `assistant_thread` staple, classified the event as side-pane, and posted "i live in #sam — talk to me there, not here" in response to a legitimate @sam in #team-engineering. Fix: short-circuit on `channel in self.member_channels` BEFORE checking the `assistant_thread` field. Real workspace channels are never side- panes regardless of metadata. PR #79's multi-channel routing depends on this — without the fix, every channel besides #sam would still get the redirect. 5 new tests pin the behavior across genuine side-panes, member channels with stapled metadata, known assistant-thread channels, unknown channels, and empty events.
New section under "Pre-push self-checks": when something breaks, the work is identifying the class of problem and closing every door the class can come back through — not just patching the visible case. Four-step protocol: name the class (not the instance), find where else the class shows up, pick a durable guard (test shape / review item / process step), ship the patch AND the guard. Anchored in the 2026-05-25 side-pane misclassification: the instance was an @sam in #team-engineering getting redirected; the class was "review checked diff-correctness against one channel kind, never traced behavior under all channel kinds." Naming both is the discipline. Counter-examples explicitly called out: single-bug test, "be careful" prose, deferred ticket. Each looks like a class fix; none is.
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
1 participant
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.
What this changes
src/runtime/daemon.py_is_side_pane_eventshort-circuits onchannel in self.member_channelsBEFORE checking Slack's stapledassistant_threadmetadatatest_channel_routing.pypin the classifier across genuine side-panes, member channels with stapled metadata (the bug), known assistant-thread channels, unknown channels, and empty events_mock_daemonhelper now initializes_assistant_thread_channelsso the new tests don't trip on itThe bug (production logs, 2026-05-25)
Operator invited Sam to #team-engineering and @-mentioned. Sam replied with the side-pane redirect ("i live in #sam — talk to me there, not here") instead of responding to the mention.
Slack staples an
assistant_threadfield onto every event for assistant-type apps, including events that arrive on real workspace channels. The previous_is_side_pane_eventshort-circuit only excludedSAM_CHANNEL(and after #79's aed41ba,broadcast_channel_id) — leaving every other member channel vulnerable to misclassification.PR #79's multi-channel routing landed on main as
a09ee9dbut the side-pane handler still treats every-channel-except-broadcast as side-pane. So the operator-visible behavior of #79 didn't actually work in production.The class fix
The instance is
_is_side_pane_eventmisfiring on one stapled-metadata signal. The class is "review checked diff-correctness against one channel kind, never traced behavior under all channel kinds." PR #79 changed the short-circuit fromSAM_CHANNELtobroadcast_channel_id— correct edit, wrong frame: still one named target, not the full member-channels set.So this PR ships two things in one:
_is_side_pane_event(member-channel short-circuit) + tests.src/capabilities/self-maintenance.md— a four-step protocol Sam runs when something breaks, with the side-pane bug as the worked example.The guard is the part that prevents the next variant of this class from shipping in a week.
Confidence
High. Bug reproduced in production logs, fixed deterministically with a member-channel short-circuit, 5 tests pin the corrected behavior. The self-maintenance addition is docs-only and doesn't affect runtime. All 199 tests pass.