Skip to content

fix(slack): include bot's own prior replies in cold-start thread context - #38936

Closed
temalo wants to merge 1 commit into
NousResearch:mainfrom
temalo:fix/38861-slack-cold-start-bot-replies
Closed

fix(slack): include bot's own prior replies in cold-start thread context#38936
temalo wants to merge 1 commit into
NousResearch:mainfrom
temalo:fix/38861-slack-cold-start-bot-replies

Conversation

@temalo

@temalo temalo commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Why

In _fetch_thread_context (gateway/platforms/slack.py), the bot's own prior
replies in a thread were unconditionally filtered out before injection. The
guard was sound for the active-session path (where the session history
already carries those replies and re-injecting them would be circular), but
_fetch_thread_context is only called when there is no active session for
the thread — the call site at line 2361 is wrapped in
if not self._has_active_session_for_thread(...). On that cold-start path,
dropping the bot's prior replies leaves the agent with only user messages and
no way to reconstruct what it (or a sibling cron session that posted the
thread parent) already said.

Concrete failure mode: a cron job posts a thread parent → a user replies in
the thread → a different agent session warms up → the agent sees user turns
only, has to guess at the prior conversation, and often re-asks for
information the bot already provided.

Fixes #38861.

What

gateway/platforms/slack.py_fetch_thread_context now keeps self-bot
child replies and labels them with an explicit [assistant] prefix instead of
continue-ing past them. The per-workspace bot-id resolution
(self._team_bot_user_ids.get(msg_team) or self._bot_user_id) is preserved
verbatim, so the existing multi-workspace correctness (only our bot in
this workspace is treated as self) still holds.

Detection logic moves into a single boolean (is_self_bot_reply); the prefix
ladder becomes [thread parent] | [assistant] | ""; user-name resolution is
skipped for self-bot turns (the [assistant] label already communicates
authorship and the resolved name would just be our own bot handle).

Behaviour change footprint

Path Before After
Cold-start cache miss, self-bot child reply present Dropped Kept, prefixed [assistant]
Cold-start, third-party bot child reply Kept (unchanged) Kept (unchanged)
Cold-start, bot-posted parent Kept, prefixed [thread parent] (unchanged) Kept, prefixed [thread parent] (unchanged)
Cold-start, user message Kept (unchanged) Kept (unchanged)
Active session path (_has_active_session_for_thread true) Not called (unchanged) Not called (unchanged)
Per-workspace self-bot detection Uses self._team_bot_user_ids per msg_team (unchanged) Uses self._team_bot_user_ids per msg_team (unchanged)

The active-session circular-context concern that originally motivated the
filter does not apply here — that path simply never reaches this method.

Test coverage

tests/gateway/test_slack_approval_buttons.py:

Test Covers
test_includes_self_bot_replies_as_assistant_on_cold_start (new behaviour) Self-bot reply kept with [assistant] prefix, third-party bot kept, [assistant] label does not leak to user lines
test_fetch_thread_context_includes_self_bot_replies_with_assistant_label (renamed/updated from _excludes_self_bot_replies) Bot-posted parent kept with [thread parent], self-bot child kept with [assistant], user replies kept
test_fetch_thread_context_multi_workspace (updated) Per-workspace self-bot detection still uses T2's bot id (not T1's); T2's self-bot reply is included with [assistant] rather than dropped; T1's bot id appearing in a T2 message is treated as third-party and kept by its display name
test_fetches_and_formats_context (unchanged, regression guard) User messages and bot-mention stripping still work
test_fetch_thread_context_includes_bot_parent (unchanged, regression guard) Bot-posted parent still labelled [thread parent]
test_fetch_thread_context_current_ts_excluded (unchanged, regression guard) The currently-triggering message is still excluded
test_empty_thread, test_api_failure_returns_empty (unchanged) Boundary cases unchanged

Verification on this branch:

pytest tests/gateway/test_slack_approval_buttons.py
  23 passed in 0.44s
pytest tests/gateway/
  6156 passed, 38 failed, 76 skipped

The 38 failures are all pre-existing on upstream/main (telegram/whatsapp
adapters, status-command onboarding) — diffed against a pristine
upstream/main baseline run, the set of failures is bit-for-bit identical.
No new failures introduced by this change.

Out of scope

  • The active-session path (_thread_context_cache reuse during a live
    session) — not touched; that path was already correct.
  • _fetch_thread_parent_text — same cache, but only ever returns the parent
    text; the self-bot filter never applied there.
  • Restructuring the thread-context cache or the per-workspace bot-id store.
  • Other platform adapters (discord, slack-via-events-only paths, etc.) —
    out of scope; the issue and its repro are Slack-specific.
  • Treating [assistant] lines as actual assistant-role messages in the
    agent's message list. Cold-start context is injected as a single user-role
    preamble today; promoting individual lines back into typed turns would be
    a broader gateway refactor and a separate change.

_fetch_thread_context unconditionally dropped self-bot child replies before
injection. The guard makes sense on the active-session path (where the
session history already carries those replies and re-injecting them would
be circular), but this method is only called from the cold-start branch —
the call site is wrapped in 'if not _has_active_session_for_thread(...)'.
On that path, dropping the bot's prior replies leaves the agent with user
messages only and no way to reconstruct what it (or a sibling cron session
that posted the parent) already said.

Keep self-bot child replies on the cold-start path and label them with an
explicit [assistant] prefix so the agent can distinguish its own prior
turns from user messages and from third-party bot posts. Per-workspace
self-bot detection (self._team_bot_user_ids per msg_team) is preserved
verbatim. User-name resolution is skipped for self-bot turns — the
[assistant] label already communicates authorship.

Test coverage in tests/gateway/test_slack_approval_buttons.py:
- new: test_includes_self_bot_replies_as_assistant_on_cold_start
- updated: test_fetch_thread_context_includes_self_bot_replies_with_assistant_label
- updated: test_fetch_thread_context_multi_workspace (per-workspace filter
  still applied, just no longer drops the self reply)
- regression guards left intact: parent labelling, bot-mention stripping,
  current_ts exclusion, empty/error paths

Fixes NousResearch#38861
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery platform/slack Slack app adapter labels Jun 4, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for identifying a live cold-start context gap. Current main still skips self-bot child replies at plugins/platforms/slack/adapter.py:3775-3786, while the only production fetch path is guarded by the absence of a thread session at plugins/platforms/slack/adapter.py:2886-2900.

Problems

  • The PR targets the pre-migration path gateway/platforms/slack.py. Slack moved to plugins/platforms/slack/adapter.py in 5600105478ffde29d7566b45421b100eaa29c4ef, and the current method now also preserves non-bot authorization tags at plugins/platforms/slack/adapter.py:3803-3815.

Suggested changes

  • During salvage, port the self-bot [assistant] branch into the current plugin method while retaining the current trust-tag handling for all non-self messages. The existing updated tests are the right behavioral coverage to carry forward.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 14, 2026
teknium1 pushed a commit that referenced this pull request Jul 22, 2026
_fetch_thread_context unconditionally filtered out the bot's own prior
replies, so cold-start sessions (bot posts a thread root, user replies
later, no active session) lost every assistant turn and the agent could
not reconstruct the prior conversation.

The circular-context concern the filter guarded against does not apply
here: the call site is gated by _has_active_session_for_thread, so this
method only runs when there is no session history to duplicate.

Self-bot replies are now kept and labelled with an explicit [assistant]
prefix (skipping user-name resolution — the label already communicates
authorship). Third-party bot posts and the bot-authored thread parent
keep their existing treatment.

Fixes #38861.

Salvaged from #38936 by @temalo, rebased from the pre-plugin
gateway/platforms/slack.py layout onto plugins/platforms/slack/adapter.py
(preserving the [unverified] trust-tag handling added on main since).
teknium1 pushed a commit that referenced this pull request Jul 22, 2026
_fetch_thread_context unconditionally filtered out the bot's own prior
replies, so cold-start sessions (bot posts a thread root, user replies
later, no active session) lost every assistant turn and the agent could
not reconstruct the prior conversation.

The circular-context concern the filter guarded against does not apply
here: the call site is gated by _has_active_session_for_thread, so this
method only runs when there is no session history to duplicate.

Self-bot replies are now kept and labelled with an explicit [assistant]
prefix (skipping user-name resolution — the label already communicates
authorship). Third-party bot posts and the bot-authored thread parent
keep their existing treatment.

Fixes #38861.

Salvaged from #38936 by @temalo, rebased from the pre-plugin
gateway/platforms/slack.py layout onto plugins/platforms/slack/adapter.py
(preserving the [unverified] trust-tag handling added on main since).
@teknium1

Copy link
Copy Markdown
Contributor

Merged via #69320 — your commit was cherry-picked onto current main with your authorship preserved in git history: your cold-start bot-reply inclusion was reapplied onto the plugin adapter with your authorship, preserving main's [unverified] trust-tag logic.

Thanks for the contribution!

@teknium1 teknium1 closed this Jul 22, 2026
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
_fetch_thread_context unconditionally filtered out the bot's own prior
replies, so cold-start sessions (bot posts a thread root, user replies
later, no active session) lost every assistant turn and the agent could
not reconstruct the prior conversation.

The circular-context concern the filter guarded against does not apply
here: the call site is gated by _has_active_session_for_thread, so this
method only runs when there is no session history to duplicate.

Self-bot replies are now kept and labelled with an explicit [assistant]
prefix (skipping user-name resolution — the label already communicates
authorship). Third-party bot posts and the bot-authored thread parent
keep their existing treatment.

Fixes NousResearch#38861.

Salvaged from NousResearch#38936 by @temalo, rebased from the pre-plugin
gateway/platforms/slack.py layout onto plugins/platforms/slack/adapter.py
(preserving the [unverified] trust-tag handling added on main since).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists platform/slack Slack app adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Slack] Bot replies excluded from thread context on cold-start sessions

3 participants