Skip to content

fix(slack): thread-scope chat session + per-speaker attribution + sender-filtered memory (#903)#1458

Merged
vybe merged 6 commits into
devfrom
AndriiPasternak31/issue-903
Jul 6, 2026
Merged

fix(slack): thread-scope chat session + per-speaker attribution + sender-filtered memory (#903)#1458
vybe merged 6 commits into
devfrom
AndriiPasternak31/issue-903

Conversation

@AndriiPasternak31

@AndriiPasternak31 AndriiPasternak31 commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Fixes #903

Problem

SlackAdapter.get_session_identifier keyed channel sessions on team:sender:channel, so every user in a channel got a separate session and two concurrent threads collided into one. The issue's one-line suggestion (drop sender_id) fixes conversation scope but silently breaks two other concerns the single key had been conflating: speaker attribution (multi-participant history would replay as a flat User:) and per-user memory source (summarize_user_memory_background reads the whole session and writes to the current user's durable MEM-001 profile — Bob replying in Alice's thread would persist Alice's turns into Bob's memory, a cross-user PII leak).

Fix

Re-key channel sessions to team:channel:thread (DMs keep team:sender:channel) and move who-said-what / whose-memory onto each message row via two nullable public_chat_messages columns:

  • sender_label — display name; history replays attributed (Alice: / Bob:), role-label fallback when null (DMs / pre-migration rows).
  • sender_email — verified speaker; the MEM-001 summarizer filters on the current user's own turns so a shared thread never feeds one user's turns into another's durable memory.

Concurrent get_or_create_session on a brand-new thread key is race-guarded (savepoint + IntegrityError re-SELECT). No migration of pre-existing channel-keyed rows (one-time "forget", expected).

Follow-up refinements in this branch

  • Assistant-turn memory parity across ALL channel DMs. A single-participant session must stamp the assistant reply with the recipient's email too, or its replies vanish from that user's filtered memory (a pre-Slack: scope chat session to thread, not channel #903 regression). The signal is per-adapter — Slack sets is_dm, groups set is_group, WhatsApp (DM-only) sets neither — so _assistant_sender_email() combines both: every DM stamps, only a Slack channel thread / group chat stays null.
  • Speaker-label sanitization (_sanitize_label) — collapse newlines/control chars before a channel-controlled name lands in the transcript's structural {speaker}: position, so a crafted display name can't forge an Assistant: line.

Migration (dual-track, #1183)

  • SQLite: public_chat_messages_sender in db/migrations.py
  • PostgreSQL: Alembic 0013_public_chat_messages_sender (chains off 0012)
  • DDL in db/schema.py, MetaData in db/tables.py, model in db_models.py

Tests

tests/unit/test_903_slack_session_thread.py (session keying, label sanitization, per-channel assistant-stamp matrix) + tests/unit/test_903_public_chat_sender.py (sender-filtered DB reads, single-participant assistant-in-feed parity). All 29 green; 193 adjacent router/adapter tests unaffected.

Docs

architecture.md, requirements/public-access.md (SLACK-002), feature-flows/slack-channel-routing.md, feature-flows/public-agent-links.md (+ index row), learnings.md.

🤖 Generated with Claude Code

AndriiPasternak31 and others added 5 commits July 4, 2026 19:41
@AndriiPasternak31 @claude
fix(slack): scope channel chat session to thread, not channel (#903)
61b6146 
Slack channel sessions were keyed team_id:sender_id:channel_id, so every
@mention and thread reply from one user in one channel shared a single
channel-wide transcript. Two concurrent threads cross-contaminated and a
fresh top-level @mention inherited the channel's last-10-turn history.

get_session_identifier now branches on is_dm: DMs keep the sender+channel
key (one continuous per-user convo, no threads); channel messages key on
team_id:channel_id:thread_id. sender_id is dropped for channels so
multi-participant threads share context — per-speaker attribution comes
from the #350 identity prefix. thread_id is already populated by the
parsers (_parse_mention mints thread_ts or ts, so a top-level mention gets
a fresh id for free). Opaque session key: zero schema change; old
channel-keyed rows orphan naturally.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@AndriiPasternak31 @claude
fix(slack): thread-scope session + per-speaker attribution + sender-f…
c6486e7 
…iltered memory (#903)

Slack channel sessions were keyed team:sender:channel, collapsing every
@mention and thread reply in a channel into one transcript — concurrent
threads cross-contaminate and a fresh @mention inherits stale history. The
single opaque session key was overloaded: it silently encoded conversation
scope, speaker attribution, and per-user memory source, which a single-user
session kept accidentally aligned.

Decompose the three axes (plan Option C):
- Key: channels use team:channel:thread (sender_id dropped so multi-participant
  threads share context; DMs keep team:sender:channel). F-TEAMID: `or "unknown"`.
- Attribution: new nullable public_chat_messages.sender_label — build_context_prompt
  replays each turn as `Alice:`/`Bob:` instead of a flat `User:` (role fallback
  when null). Set from the #350 enrich metadata on the channel user turn.
- Per-user memory: new nullable sender_email — the MEM-001 summarizer filters
  get_recent_public_chat_messages by the current user's email so a shared thread
  never feeds one user's turns into another's durable memory (F-MEM). Web path
  stamps sender_email too so the filter is a no-op there.

F-RACE: thread-scoping newly lets two different users race the select-then-insert
on a brand-new thread key; wrap the INSERT in a savepoint + IntegrityError
re-SELECT so the loser converges on the winner's row instead of 500ing.

Dual-track schema (Invariant #3): schema.py DDL + tables.py MetaData +
db/migrations.py SQLite ALTER + Alembic 0013 + db_models.PublicChatMessage.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@AndriiPasternak31 @claude
docs(slack): record thread-scoped session + attribution + sender-filt…
e5ae0ac 
…ered memory (#903)

- requirements/public-access.md: SLACK-002 §15.1b-ii key feature + sender columns
- architecture.md: message_router line — history attributed per stored sender,
  MEM-001 summarization sender-filtered
- feature-flows/slack-channel-routing.md: session-identifier (DM vs thread),
  per-speaker attribution & memory section, updated test checklist + unit refs
- feature-flows.md: Recent Updates row (trim oldest to keep the ~20 cap)
- learnings.md: overloaded-session-key pattern (split scope/attribution/memory)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@ap-31 @claude
docs(memory): note sender-filtered summarizer read in public-agent-li…
fb4028d 
…nks (#903)

The every-5-message MEM-001 summarizer now reads
get_recent_public_chat_messages(session_id, sender_email=user_email); reflect
the #903 filter in the shared memory-summarization flow doc.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@ap-31 @claude
fix(slack): stamp assistant turn for all single-participant DMs + san…
de96b24 
…itize speaker label (#903)

Two follow-ups to the #903 sender-filtered MEM-001 memory work:

1. Assistant-turn memory parity across ALL channel DMs. The summarizer reads
   get_recent_public_chat_messages(sender_email=user_email); a single-participant
   session must stamp the assistant reply with the recipient's email too, or its
   replies vanish from that user's durable memory. The first pass keyed on the
   Slack-only `is_dm` metadata key, which regressed Telegram DMs (they set
   `is_group`, never `is_dm`) and ALL WhatsApp chats (DM-only; sets neither) —
   the exact pre-#903 behavior the fix meant to preserve. Extract
   `_assistant_sender_email()` combining each adapter's own signal (Slack `is_dm`,
   group `is_group`) so every DM stamps and only a Slack channel thread / group
   chat stays null. Web sync + background paths stamp unconditionally (always
   single-participant).

2. Sanitize the channel-controlled speaker label (`_sanitize_label`) — collapse
   newlines/control chars before the label lands in the replayed transcript's
   structural `{speaker}:` position, so a crafted display name can't forge an
   `Assistant:` line (defence in depth; a no-op for legitimate names).

Regression tests: per-channel assistant-stamp matrix (Slack DM/channel, Telegram
DM/group, WhatsApp DM), single-participant assistant-in-feed parity, and label
newline stripping. Docs (slack-channel-routing, public-agent-links) generalized
from "Slack DMs" to "any channel DM".

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@AndriiPasternak31
AndriiPasternak31 marked this pull request as ready for review July 4, 2026 21:47
@AndriiPasternak31
AndriiPasternak31 requested review from dolho and vybe July 4, 2026 21:47
@AndriiPasternak31 AndriiPasternak31 self-assigned this Jul 4, 2026
@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown

⚠️ Nightly unit-suite check skipped — merge conflict against dev.

Resolve by running git merge dev locally and pushing the result. The next nightly run will re-test once the conflict is gone.

@claude
Merge origin/dev into issue-903 — keep both learnings entries and all…
7f5ef4c 
… Recent-Updates rows

database.py overlap with #1445 auto-merged; 37 affected unit tests pass.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
vybe
vybe approved these changes Jul 6, 2026
View reviewed changes

@vybe vybe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Validated via /validate-pr: conventional commits, Fixes #903 (P2 type-bug, theme-channels), dual-track migration verified (SQLite public_chat_messages_sender + Alembic 0013 chaining off 0012 head, schema.py/tables.py/db_models.py consistent), 29 tests incl. thread-vs-channel scoping, race guard, and sender-filtered memory (closes a cross-user memory-write leak). Dev-merge conflicts (learnings.md, feature-flows.md) resolved keep-both; database.py overlap with #1453 auto-merged, 37 affected unit tests pass locally. Note: open PR #1339 will need its Alembic revision renumbered after this lands.

@vybe
vybe enabled auto-merge (squash) July 6, 2026 09:05
@vybe
vybe merged commit 3f2d5aa into dev Jul 6, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vybe vybe vybe approved these changes
@dolho dolho Awaiting requested review from dolho

Assignees

@AndriiPasternak31 AndriiPasternak31

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@AndriiPasternak31 @vybe @ap-31