Skip to content

fix(slack): rehydrate thread context after restart - #33215

Closed
vexclawx31 wants to merge 1 commit into
NousResearch:mainfrom
vexclawx31:fix/slack-thread-context-rehydration
Closed

fix(slack): rehydrate thread context after restart#33215
vexclawx31 wants to merge 1 commit into
NousResearch:mainfrom
vexclawx31:fix/slack-thread-context-rehydration

Conversation

@vexclawx31

@vexclawx31 vexclawx31 commented May 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

  • port restart rehydration from the obsolete Slack adapter into plugins/platforms/slack/adapter.py on current main
  • rehydrate an active persistent thread session once per gateway process/session scope, marking success only after non-empty context
  • preserve shared-thread dedupe and thread_sessions_per_user isolation
  • refresh Slack context on explicit thread mentions (including strict-mention mode) while preventing repeated full-thread injection on ordinary follow-ups
  • bypass the short-lived thread-context cache for explicit mentions so newer replies are visible

Test Plan

  • .venv/bin/python -m pytest tests/gateway/test_slack.py::TestSlackThreadContextRehydration tests/gateway/test_slack_approval_buttons.py::TestSlackThreadContext::test_api_empty_thread_is_not_cached_and_retries tests/gateway/test_slack_approval_buttons.py::TestSlackThreadContext::test_force_refresh_bypasses_nonempty_context_cache -q -o 'addopts=' — 8 passed
  • .venv/bin/python -m pytest tests/gateway/test_slack*.py -q -o 'addopts=' — 449 passed (52 existing runtime warnings)
  • .venv/bin/ruff check plugins/platforms/slack/adapter.py tests/gateway/test_slack.py tests/gateway/test_slack_approval_buttons.py — passed
  • git diff --check — passed

Notes

This branch was rebuilt on current upstream main; the obsolete gateway/platforms/slack.py changes are no longer present.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists platform/slack Slack app adapter comp/gateway Gateway runner, session dispatch, delivery labels May 27, 2026
teknium1
teknium1 reviewed Jul 13, 2026
View reviewed changes

@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 isolating the restart rehydration case and covering retry and session-scope variants.

Problems

  • The PR edits gateway/platforms/slack.py, but commit 5600105478ffde29d7566b45421b100eaa29c4ef migrated the live adapter to plugins/platforms/slack/adapter.py. Current main still has the active-session guard that suppresses hydration at plugins/platforms/slack/adapter.py:2888-2900, so this needs a targeted port rather than applying the old-path diff.

Suggested changes

  • Reapply the marker-based one-shot hydration logic in plugins/platforms/slack/adapter.py and relocate the regression tests to the current test layout. Preserve the current routing changes around plugins/platforms/slack/adapter.py:2819-2868 while doing so.

Automated hermes-sweeper review.

Comment thread gateway/platforms/slack.py Outdated
# a restarted gateway can treat an incomplete/sparse session as
# authoritative and only inject the thread parent quote. Fetching
# once per process/thread rehydrates Slack-side context without
# prepending the full thread on every reply. Entries are added only

@teknium1 teknium1 Jul 13, 2026

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.

Current main no longer loads this adapter path: commit 5600105478ffde29d7566b45421b100eaa29c4ef moved it to plugins/platforms/slack/adapter.py. Please port this state and the call-site change to that live plugin adapter.

@teknium1 teknium1 added 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 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 13, 2026
@vexclawx31
vexclawx31 force-pushed the fix/slack-thread-context-rehydration branch from 764f70c to f08cfb5 Compare July 19, 2026 18:01
@vexclawx31

vexclawx31 commented Jul 19, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the review in f08cfb54c3509ba7b10ac512bc0db38462324d58.

  • rebuilt the existing branch on current main
  • moved the restart marker logic into plugins/platforms/slack/adapter.py; the obsolete adapter/test-path diff is gone
  • preserved the current mention/session routing gates
  • added one-successful-rehydration markers with shared vs per-user thread scoping and retry-on-empty behavior
  • kept explicit mentions (including strict mode) as a forced context refresh, while ordinary active-session replies inject the full thread only once after restart

Verification: focused regressions 8 passed; all tests/gateway/test_slack*.py 449 passed; Ruff and git diff --check passed.

@teknium1 teknium1 mentioned this pull request Jul 22, 2026
Merged
teknium1 pushed a commit that referenced this pull request Jul 22, 2026
@vexclawx31 @teknium1
fix(slack): rehydrate thread context after gateway restart
c6a7f20 
Persistent sessions survive gateway restarts, but thread replies posted
while the gateway was DOWN never reached the session — and the adapter
had no way to notice, so the conversation silently resumed with a hole
in it.

On the first ordinary reply per thread after a restart (tracked by a
fresh-process _thread_rehydration_checked set), fetch the thread delta
past the persisted per-session watermark and inject any missed messages
as part of the new turn via channel_context. Exactly-once per thread
per process; when the watermark is empty (pre-feature sessions) the
check is a no-op. Steady-state replies keep advancing the watermark so
rehydration never re-injects messages the session already carries as
ordinary turns. Prior history is never rewritten (prompt caching safe).

Builds on the persisted watermark introduced for #23918.

Salvaged from #33215 by @vexclawx31, reworked from a repeated
full-thread injection guard into a watermark-delta injection so
rehydration adds only what the session actually missed.
teknium1 added a commit that referenced this pull request Jul 22, 2026
@teknium1
chore(contributors): add email mappings for slack thread-lifecycle sa…
3ca6ea1 
…lvage

LevSky22 (#66069), vexclawx31 (#33215), knoal (#64067), kaiyisg (#24848).
teknium1 pushed a commit that referenced this pull request Jul 22, 2026
@vexclawx31 @teknium1
fix(slack): rehydrate thread context after gateway restart
fc0009b 
Persistent sessions survive gateway restarts, but thread replies posted
while the gateway was DOWN never reached the session — and the adapter
had no way to notice, so the conversation silently resumed with a hole
in it.

On the first ordinary reply per thread after a restart (tracked by a
fresh-process _thread_rehydration_checked set), fetch the thread delta
past the persisted per-session watermark and inject any missed messages
as part of the new turn via channel_context. Exactly-once per thread
per process; when the watermark is empty (pre-feature sessions) the
check is a no-op. Steady-state replies keep advancing the watermark so
rehydration never re-injects messages the session already carries as
ordinary turns. Prior history is never rewritten (prompt caching safe).

Builds on the persisted watermark introduced for #23918.

Salvaged from #33215 by @vexclawx31, reworked from a repeated
full-thread injection guard into a watermark-delta injection so
rehydration adds only what the session actually missed.
teknium1 added a commit that referenced this pull request Jul 22, 2026
@teknium1
chore(contributors): add email mappings for slack thread-lifecycle sa…
17155e3 
…lvage

LevSky22 (#66069), vexclawx31 (#33215), knoal (#64067), kaiyisg (#24848).
@teknium1

teknium1 commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Merged via #69320 — your commit was cherry-picked onto current main with your authorship preserved in git history: your restart rehydration landed via the watermark delta mechanism with your authorship.

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
@vexclawx31 @teknium1
fix(slack): rehydrate thread context after gateway restart
00935e3 
Persistent sessions survive gateway restarts, but thread replies posted
while the gateway was DOWN never reached the session — and the adapter
had no way to notice, so the conversation silently resumed with a hole
in it.

On the first ordinary reply per thread after a restart (tracked by a
fresh-process _thread_rehydration_checked set), fetch the thread delta
past the persisted per-session watermark and inject any missed messages
as part of the new turn via channel_context. Exactly-once per thread
per process; when the watermark is empty (pre-feature sessions) the
check is a no-op. Steady-state replies keep advancing the watermark so
rehydration never re-injects messages the session already carries as
ordinary turns. Prior history is never rewritten (prompt caching safe).

Builds on the persisted watermark introduced for NousResearch#23918.

Salvaged from NousResearch#33215 by @vexclawx31, reworked from a repeated
full-thread injection guard into a watermark-delta injection so
rehydration adds only what the session actually missed.
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
@teknium1
chore(contributors): add email mappings for slack thread-lifecycle sa…
28f19bf 
…lvage

LevSky22 (NousResearch#66069), vexclawx31 (NousResearch#33215), knoal (NousResearch#64067), kaiyisg (NousResearch#24848).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@teknium1 teknium1 teknium1 left review comments

Assignees

No one assigned

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-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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@vexclawx31 @teknium1 @alt-glitch