Skip to content

fix(slack): use generated assistant thread titles - #74385

Open
ghostmfr wants to merge 1 commit into
NousResearch:mainfrom
ghostmfr:fix/slack-semantic-thread-titles
Open

fix(slack): use generated assistant thread titles#74385
ghostmfr wants to merge 1 commit into
NousResearch:mainfrom
ghostmfr:fix/slack-semantic-thread-titles

Conversation

@ghostmfr

@ghostmfr ghostmfr commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Slack Agent/Assistant DM threads were titled from the raw first user message even though Hermes already generates a concise semantic session title after an early completed exchange.

This change removes the raw-prompt title mutation and best-effort sends Hermes's generated session title to Slack instead.

Related Issue

No linked issue.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • Remove eager Slack thread titling from inbound raw user prompts.
  • Route the generated Hermes session title through the current TurnRunner auto-title callback.
  • Restrict title callbacks to Slack Agent/Assistant DM lifecycle provenance; ordinary threaded IMs and MPIMs do not enter this lane.
  • Fail closed for stale sessions, unknown workspaces, disabled configuration, malformed or non-finite timestamps, and threads predating the current gateway process.
  • Distinguish explicit run invalidation (/new, /stop, stale-run eviction) from ordinary later turns with a small in-memory marker in the existing session state, closing pre-rotation and late-auxiliary-callback races without suppressing the first valid generated title.
  • Reserve a bounded workspace/channel/thread key before Slack I/O so concurrent duplicates and ambiguous Slack failures do not cause repeated attempts while the reservation remains cached.
  • Keep in-flight reservations protected while trimming completed reservations, preventing both self-eviction and eviction by newer concurrent title attempts under cache pressure.
  • Keep lifecycle provenance and title reservations workspace-scoped and bounded.
  • Preserve Telegram and Discord title callback behavior.
  • Document the bounded, best-effort contract honestly: restarts, stale sessions, cache eviction, title-generation failure, or Slack API failure may leave Slack's default title in place.

No persistence ledger, durable schema/database change, session-history lookup, or route metadata is added.

How to Test

  1. Configure Slack Agent/Assistant DMs with assistant_thread_titles: true.
  2. Start a new Slack Agent/Assistant DM thread and complete the first exchange.
  3. Confirm Slack changes from its default title to Hermes's generated semantic title rather than the raw prompt.
  4. Confirm later turns do not immediately repeat the title call while the bounded reservation remains cached.

Automated verification on current upstream main:

scripts/run_tests.sh tests/gateway/test_slack.py tests/gateway/test_fast_command.py tests/gateway/test_stop_thread_sibling.py -q
# 193 passed

scripts/run_tests.sh tests/gateway/ -q
# 4,455 passed; 4 failed
# The same four failures reproduce unchanged on the exact clean upstream base:
# - shutdown diagnostic subprocess environment (1)
# - Linux abstract systemd socket unsupported on macOS (1)
# - optional WeCom XML dependency unavailable in this local environment (2)

ruff check gateway/run.py gateway/session_state.py gateway/slash_commands.py \
  plugins/platforms/slack/adapter.py tests/gateway/test_fast_command.py \
  tests/gateway/test_slack.py tests/gateway/test_stop_thread_sibling.py
# All checks passed

python -m py_compile gateway/run.py gateway/session_state.py gateway/slash_commands.py \
  plugins/platforms/slack/adapter.py tests/gateway/test_fast_command.py \
  tests/gateway/test_slack.py tests/gateway/test_stop_thread_sibling.py
# passed

git diff --cached --check
# passed

RED-capability checks: applying the original feature tests to the exact clean upstream base produced 20 expected failures. Three later adversarial regressions also failed first—active reservation eviction under cache pressure, reset invalidation before session-ID rotation, and /stop with no active agent leaving a pending title authorized—then passed after focused fixes. The complete candidate passed all 193 focused tests.

A live macOS Slack Agent/Assistant DM smoke test on the reviewed predecessor candidate also confirmed generated semantic titling and normal response streaming before the change was replayed cleanly onto current upstream main.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 26.3

The focused candidate suites are green. The broader gateway suite has four current-main baseline failures in unrelated files; GitHub CI remains the authoritative supported-environment full run.

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A (no config key changed)
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Screenshots / Logs

No screenshot included. The behavior was verified live in a Slack Agent/Assistant DM and through focused deterministic regressions.

@ghostmfr
ghostmfr force-pushed the fix/slack-semantic-thread-titles branch from 866dd45 to 731273f Compare July 29, 2026 21:02
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/gateway Gateway runner, session dispatch, delivery comp/plugins Plugin system and bundled plugins platform/slack Slack app adapter sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Jul 29, 2026

@teknium1 teknium1 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.

Thanks for tracing the generated-title path and covering stale-session/workspace cases.

Problems

  • plugins/platforms/slack/adapter.py:1015 adds _claimed_assistant_thread_titles as an unbounded process-lifetime set. Every successfully authorized DM title permanently retains a workspace/channel/thread tuple. Current main bounds the prior _titled_assistant_threads guard at 5,000 entries (plugins/platforms/slack/adapter.py:4592-4602); this new guard has no equivalent lifecycle.
  • The PR is currently dirty against main. The generated-title callback moved into TurnRunner (gateway/run.py:5298-5354) after the PR base, so the Slack callback needs conflict-aware transplanting into that current chain.

Suggested changes

  • Preserve at-most-once behavior with bounded or durable ownership state, and cover the high-thread-count lifecycle.
  • Salvage the callback using the current TurnRunner context values (ctx.source, effective_session_id, session_key, and run_generation).

Automated hermes-sweeper review.

Comment thread plugins/platforms/slack/adapter.py Outdated
self._titling_assistant_threads: set = set()
self._attempted_assistant_thread_titles: set = set()
# Exact ownership claims are intentionally retained for this process's
# lifetime. Evicting them would reauthorize an unconditional remote

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.

_claimed_assistant_thread_titles is intentionally retained for the entire process and has no cap or cleanup path, unlike the existing 5,000-entry _titled_assistant_threads guard. A long-running gateway will retain one tuple for every titled DM thread. Please use bounded or durable ownership state that keeps the at-most-once guarantee, and add a high-thread-count lifecycle test.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The PR is updated to address both points.

  • Replayed the change cleanly onto current main and integrated it through the existing TurnRunner context (ctx.source, effective_session_id, session_key, and the session invalidation generation).
  • Replaced the unbounded ownership set with bounded, workspace-scoped best-effort state. In-flight reservations are protected from cache-pressure eviction, and ambiguous Slack attempts remain claimed to avoid duplicate calls.
  • Added coverage for high-thread-count pressure, concurrent duplicates, /new and /stop invalidation, ordinary later turns, malformed timestamps, workspace isolation, and Agent/Assistant DM provenance.

Would appreciate another look when you have a chance.

@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 30, 2026
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 comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have platform/slack Slack app adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages 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.

3 participants