fix(slack): preserve thread context for commands - #66069
Conversation
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Comment
Scope: New skill addition
Notes
- Adds a new skill to the repository.
- Skill-only additions (no core code changes).
- No security concerns, no debug artifacts.
- LGTM.
Reviewed by Hermes Agent
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating the thread-backfill mutation; current main does prepend history before command parsing at plugins/platforms/slack/adapter.py:3410-3417, so the reported defect is real.
Problems
- The new
channel_contextassignment does not protect the command from earlier enrichment: Block Kit extraction/serialization mutatestextatplugins/platforms/slack/adapter.py:3163-3185, and text-file injection can prepend content at:3640-3644. A command can therefore still get altered arguments or lose its leading slash. - The active
/queuepath rebuilds its follow-up event atgateway/run.py:9698-9715withoutchannel_context; the queued turn only consumes context from that clone at:20741-20746. This loses the recovered history the PR intends to retain.
Suggested changes
- Carry a canonical recognized command through enrichment, and copy
channel_contextthrough the/queuefollow-up event. Add a gateway-path regression rather than only asserting the adapter event.
Automated hermes-sweeper review.
| @@ -3414,7 +3420,7 @@ async def _handle_slack_message( | |||
| team_id=team_id, | |||
There was a problem hiding this comment.
Keeping thread history separate is necessary, but it does not protect commands from prior Slack enrichment: Block Kit processing mutates text at :3163-3185 and text-file injection can prepend at :3640-3644. Preserve a canonical recognized command through those paths as well, or !queue can still gain unintended arguments or lose its leading slash.
There was a problem hiding this comment.
Thanks, the remaining Block Kit/file-enrichment and queued-event context propagation concerns appear to be addressed by #66310, which explicitly consolidates this PR’s thread-backfill fix with the broader command-integrity path. Since the implementations now overlap substantially, should #66069 remain as the focused alternative, or should it be closed in favor of #66310?
Prepending cold-start thread backfill directly onto the message text
moved a recognized command (e.g. a bang-normalized "!queue ...") away
from character zero, so downstream command routing misclassified it as
conversational text and the command silently didn't run.
Route the backfill through MessageEvent.channel_context instead —
gateway.run already prepends channel_context after command dispatch
("[New message]" framing), so commands keep their COMMAND type while
the recovered history stays available to the agent.
Supersedes #68020, which dropped the fetched context entirely for
commands instead of preserving it out-of-band.
Salvaged from #66069 by @LevSky22.
Prepending cold-start thread backfill directly onto the message text
moved a recognized command (e.g. a bang-normalized "!queue ...") away
from character zero, so downstream command routing misclassified it as
conversational text and the command silently didn't run.
Route the backfill through MessageEvent.channel_context instead —
gateway.run already prepends channel_context after command dispatch
("[New message]" framing), so commands keep their COMMAND type while
the recovered history stays available to the agent.
Supersedes #68020, which dropped the fetched context entirely for
commands instead of preserving it out-of-band.
Salvaged from #66069 by @LevSky22.
|
Merged via #69320 — your commit was cherry-picked onto current main with your authorship preserved in git history: your command thread-context preservation landed via the MessageEvent.channel_context route with your authorship. Thanks for the contribution! |
Prepending cold-start thread backfill directly onto the message text
moved a recognized command (e.g. a bang-normalized "!queue ...") away
from character zero, so downstream command routing misclassified it as
conversational text and the command silently didn't run.
Route the backfill through MessageEvent.channel_context instead —
gateway.run already prepends channel_context after command dispatch
("[New message]" framing), so commands keep their COMMAND type while
the recovered history stays available to the agent.
Supersedes NousResearch#68020, which dropped the fetched context entirely for
commands instead of preserving it out-of-band.
Salvaged from NousResearch#66069 by @LevSky22.
…lvage LevSky22 (NousResearch#66069), vexclawx31 (NousResearch#33215), knoal (NousResearch#64067), kaiyisg (NousResearch#24848).
What does this PR do?
Keeps normalized Slack command text routable when Hermes restores context for the first message in an existing thread after a gateway restart.
Slack thread replies use
!commandbecause Slack intercepts native/commandtext. Hermes normalizes a known bang command such as!queue laterto/queue later, but first-entry thread backfill is currently prepended to the command text. That moves/away from character zero, soMessageEventno longer classifies the input as a command.This change keeps the normalized command in
MessageEvent.textand carries recovered history in the existingMessageEvent.channel_contextfield. Command routing therefore sees the exact command and arguments, while the subsequent agent turn can still receive the Slack conversation history that the backfill was designed to preserve. Normal non-command thread messages retain their existing behavior.This complements #30592 rather than replacing it. #30592 re-normalizes
@bot !commandafter stripping the mention, protects command text from rich-block enrichment, and skips thread-context fetching for commands. This PR addresses a separate semantic edge: already-recognized commands such as plain!queueshould preserve first-entry thread history without mixing that history into command text. If #30592 merges first, this patch may need a small textual rebase around the same adapter block.Related Issue
Related work: #25355, #26309, #30592, #56718.
No separate issue has been filed. The behavior is covered by a focused adapter regression on current
main.Type of Change
Changes Made
plugins/platforms/slack/adapter.pyMessageEvent.channel_contextfield.tests/gateway/test_slack.py!queuecommand.How to Test
scripts/run_tests.sh tests/gateway/test_slack.py -q.scripts/check-windows-footguns.py --diff upstream/main.!queue follow up after the current task./queuewith the exact arguments and retains earlier thread messages as channel context.Checklist
Code
fix(scope):,feat(scope):, etc.)scripts/run_tests.shand all selected tests passDocumentation & Housekeeping
docs/, docstrings) — N/A; no user-facing command syntax changescli-config.yaml.exampleif I added/changed config keys — N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — N/AAttribution
!commandthread fallback was introduced by @teknium1 in feat(slack): support !cmd as alternate prefix for slash commands in threads #25355.channel_contextimplementation is independently scoped and does not copy their code.Screenshots / Logs