fix(daemon): @sam in non-broadcast member channels misclassified as side-pane

src/capabilities/self-maintenance.md

@@ -180,6 +180,29 @@ Whenever a change touches *how Sam interacts with an external system* — a new
 
 Local "it ran in docker compose" tells you the code is well-formed. It does *not* tell you the deploy will succeed or the integration will work. Different layer of question.
 
+## When something breaks — solve the class, not the instance
+
+When a bug surfaces, a review misses something, or a deploy regresses, the work is not the specific fix — it is **identifying the class of problem the instance belongs to** and closing every door the class can come back through. The single-instance fix is cheap and feels productive; the class fix is the one that prevents the next variant from shipping in a week with the same shape.
+
+This is a learned discipline, not a default. The shortest path is always "patch the visible case and move on." Don't take it.
+
+**The protocol.** When something breaks — before reaching for the patch — answer in order:
+
+1. **Name the class, not the instance.** The instance is "side-pane misclassification when @sam fires in #team-engineering." The class is "review checked diff-correctness against one channel kind, never traced behavior under all channel kinds." The class fix is harder to spot and worth more.
+2. **Where else does the class show up?** Same shape applied to other handler chains, classifier helpers, anywhere a heuristic gets fooled by stapled metadata, anywhere a rule narrows on one named target when it should narrow on the full set. Spell those locations out by name, even if Sam doesn't touch them in this PR.
+3. **What guard closes the class permanently?** A new test shape (end-to-end routing tests, not just helper unit tests). A new review checklist item (trace a concrete event through every handler, with each channel kind as input). A process step in an existing skill or capability. Pick one durable mechanism — not "be more careful."
+4. **Ship the patch AND the guard.** The patch alone is theater. The next instance of the class surfaces in a week or a month with the same shape and the same root review failure. Land the guard in the same PR or as an immediate follow-up — and *name it* in the PR description so the reviewer can grade Sam's class-naming.
+
+**Counter-examples — these look like class fixes but aren't:**
+
+- Adding a single new test for the exact bug. That's a patch with a paper trail. The next variant slips through because the *kind of check* hasn't changed.
+- Adding a prose rule that says "be careful about X." Prose Sam already ignored under load is not enforcement.
+- Filing a ticket "look into this later." Almost never revisited; the next variant hits before the ticket does.
+
+**Why it matters.** Time spent naming the class and shipping the guard is recovered the next time the class would have surfaced. Sam's review style is what produced the bug, not the buggy code itself — patching only the code leaves the review style intact. The operator's framing on 2026-05-25: *"we should never repeat the same mistakes again by trying to systematically solve the entire class of problems."* That sentence is the rule.
+
+Cross-references: `src/skills/github-pr-workflow/skill.md` ("Before pushing: review the diff as a cold reader") and the "Verify before relying" section above both compose with this principle — they describe specific classes worth guarding against; this section names the discipline of asking *"what class did this belong to?"* every time something breaks.
+
 ## What Sam does not do
 
 - **Bypass the review gate.** Sam never uses `gh pr merge` without `--auto` (which respects branch protection). No `--admin` flag, no ruleset edits, no bypass actors. The required-review gate exists for a reason — Sam leans on it.

src/runtime/daemon.py

@@ -381,19 +381,26 @@ def _catchup_target_channels(self) -> list[str]:
     def _is_side_pane_event(self, event: dict) -> bool:
         """Detect Agents & AI Apps assistant-thread (side-pane) messages.
 
-        Slack delivers side-pane messages to a separate channel id from
-        the workspace channel. So anything arriving on the whitelisted
-        SAM_CHANNEL is, by definition, NOT a side-pane event — even when
-        Slack staples an `assistant_thread` field onto it as workspace-
-        level metadata (which it does for assistant-type apps).
-
-        After that short-circuit, two signals: an `assistant_thread`
-        field on the event, or a channel id we've previously seen open
-        via `assistant_thread_started`.
+        Slack delivers side-pane messages on a SEPARATE synthetic channel
+        id (the assistant-thread channel) — never on a real workspace
+        channel id. So any event arriving on a channel Sam is a member
+        of is, by definition, NOT a side-pane event — even when Slack
+        staples an `assistant_thread` field onto it as workspace-level
+        metadata (which it does on regular-channel events for
+        assistant-type apps; verified in production logs 2026-05-25 on
+        an @-mention in #team-engineering).
+
+        After the member-channel short-circuit, two signals identify a
+        genuine side-pane event: an `assistant_thread` field on the
+        event, or a channel id we've previously seen open via
+        `assistant_thread_started`.
         """
         channel = event.get("channel")
-        target_channel = self.broadcast_channel_id
-        if target_channel and channel == target_channel:
+        # Real workspace channels Sam is a member of are not side-panes.
+        # This MUST come before the `assistant_thread`-field check because
+        # Slack staples that field onto every event for assistant-type
+        # apps regardless of channel kind.
+        if channel and channel in self.member_channels:
             return False
         if event.get("assistant_thread"):
             return True

tests/runtime/test_channel_routing.py

@@ -34,8 +34,8 @@
 def _mock_daemon():
     """Construct a Daemon without running __init__ (which would open a
     Slack socket) and wire in just the attrs the channel-routing helpers
-    read: bot_user_id, app.client, the two name<->id dicts, and the
-    member_channels set."""
+    read: bot_user_id, app.client, the two name<->id dicts, the
+    member_channels set, and the assistant-thread channel set."""
     from src.runtime.daemon import Daemon
     d = Daemon.__new__(Daemon)
     d.bot_user_id = "U0BOT"
@@ -44,6 +44,7 @@ def _mock_daemon():
     d._channel_id_by_name = {}
     d._channel_name_by_id = {}
     d.member_channels = set()
+    d._assistant_thread_channels = set()
     return d
 
 
@@ -240,3 +241,70 @@ def test_channel_allowed_rejects_None():
     d = _mock_daemon()
     assert d._channel_allowed(None) is False
     assert d._channel_allowed(None, force_allow_member=True) is False
+
+
+# ─── _is_side_pane_event — member-channel short-circuit ───────────────────────
+
+
+def test_is_side_pane_event_false_for_member_channel_even_with_assistant_thread_metadata():
+    """The headline bug observed 2026-05-25: Slack staples an
+    `assistant_thread` field onto events for assistant-type apps even
+    when the event arrived on a real workspace channel. The previous
+    `_is_side_pane_event` short-circuited only on SAM_CHANNEL (or
+    `broadcast_channel_id`), leaving every OTHER member channel
+    vulnerable — an @-mention in #team-engineering got misclassified
+    as side-pane and Sam posted "i live in #sam — talk to me there,
+    not here" instead of responding.
+
+    Fix: short-circuit on `channel in self.member_channels`. Real
+    workspace channels are never side-panes regardless of metadata.
+    """
+    d = _mock_daemon()
+    d.member_channels = {"C_SAM", "C_TEAM_ENG"}
+    event = {
+        "channel": "C_TEAM_ENG",  # real workspace channel
+        "assistant_thread": {"channel_id": "D_SIDEPANE"},  # Slack-stapled metadata
+        "text": "<@U0BOT> hello",
+    }
+    assert d._is_side_pane_event(event) is False
+
+
+def test_is_side_pane_event_true_for_synthetic_channel_with_metadata():
+    """Genuine side-pane: channel is the synthetic assistant-thread id
+    (NOT a member of the workspace channel set), `assistant_thread`
+    metadata is present. This is what the redirect is for — keep it
+    firing here."""
+    d = _mock_daemon()
+    d.member_channels = {"C_SAM", "C_TEAM_ENG"}
+    event = {
+        "channel": "D_SIDEPANE",
+        "assistant_thread": {"channel_id": "D_SIDEPANE"},
+    }
+    assert d._is_side_pane_event(event) is True
+
+
+def test_is_side_pane_event_true_for_known_assistant_thread_channel():
+    """A channel we've previously seen via `assistant_thread_started` is
+    a side-pane even without the metadata on this specific event."""
+    d = _mock_daemon()
+    d.member_channels = {"C_SAM"}
+    d._assistant_thread_channels = {"D_KNOWN_SIDEPANE"}
+    event = {"channel": "D_KNOWN_SIDEPANE"}
+    assert d._is_side_pane_event(event) is True
+
+
+def test_is_side_pane_event_false_for_unknown_channel_with_no_metadata():
+    """Unknown channel, no metadata, not in member_channels, not in
+    assistant_thread_channels — not a side-pane (the caller's other
+    gates will reject it for being non-member anyway)."""
+    d = _mock_daemon()
+    d.member_channels = {"C_SAM"}
+    event = {"channel": "C_UNKNOWN"}
+    assert d._is_side_pane_event(event) is False
+
+
+def test_is_side_pane_event_false_for_event_with_no_channel():
+    """Defensive: events without a channel field can't be side-pane."""
+    d = _mock_daemon()
+    assert d._is_side_pane_event({}) is False
+    assert d._is_side_pane_event({"assistant_thread": {}}) is False