feat(slack): add --no-assistant flag to manifest generation - #51416
feat(slack): add --no-assistant flag to manifest generation#51416victor-kyriazakos wants to merge 1 commit into
Conversation
By default `hermes slack manifest` opts the app into Slack's AI Assistant container (assistant_view feature + assistant:write scope + assistant_thread_* events). Slack then renders DMs as the right-hand Assistant split-pane, where every exchange is a thread and bare slash commands (/help, /new, ...) are not delivered as normal command events — they only work when the bot is @mentioned. There was no way to opt out short of hand-editing the generated JSON. Add --no-assistant to emit a flat-DM manifest that omits those three pieces, so DMs render as a normal chat and slash commands dispatch inline. The regular messaging surface (Messages tab, slash commands, Socket Mode, channel + DM scopes/events) is preserved in both modes. Default behaviour is unchanged (assistant mode still on). Tests: cover both manifest modes and the argparse wiring.
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved
Add --no-assistant flag to Slack manifest generation.
Looks Good
- Clean feature addition with clear documentation
- Properly handles the assistant_view feature, scopes, and events
- Default behavior preserved (include_assistant=True)
- Well-structured manifest building with conditional assistant pieces
- Tests verify both modes (with and without assistant)
Reviewed by Hermes Agent
|
Merged via #51487 — your commit was cherry-picked onto current main with your authorship preserved (rebase-merge, commit da80ac0). Verified the premise on main, kept assistant-on as the default (the adapter relies on assistant mode for the "is thinking…" status and pre-message session/memory scoping), and confirmed both manifest modes E2E. Thanks @victor-kyriazakos! |
|
Thanks for the careful review — agreed on the headline: keep assistant-on as the default, On the two specific concerns, after tracing the code + verifying against a live flat-DM install, I'd refine the framing: 1. "is thinking…" indicator → "falls back to reactions." This is narrower than it reads. There are three independent indicators with different dependencies:
So flat-DM does not collapse the thinking indicator down to a bare emoji reaction — the primary status text is scope-independent. (Screenshots from a live 2. Pre-message session/memory scoping via TL;DR: default stays assistant-on (agreed); the only things truly tied to assistant mode are the native |
Summary
hermes slack manifestalways emits a manifest that opts the app into Slack's AI Assistant container — theassistant_viewfeature, theassistant:writescope, and theassistant_thread_started/assistant_thread_context_changedevents. There is currently no flag to turn this off; the only workaround is hand-editing the generated JSON.With assistant mode on, Slack renders DMs as the right-hand Assistant split-pane, where every exchange is a thread and bare slash commands (
/help,/new, …) are not delivered as normalcommandevents — they only dispatch when the bot is@mentioned. Users hitting this report "slash commands only work when I tag the bot, even in DMs" and "DMs always spawn a thread I can't send commands to."This PR adds
--no-assistanttohermes slack manifest, emitting a flat-DM manifest that omits those three pieces. DMs then render as a normal chat and slash commands dispatch inline.Behaviour
--no-assistant— dropsassistant_view,assistant:write, and the twoassistant_thread_*events.channels:*,groups:*,message.channels,message.groups,message.im,app_mention).Usage
Tests
tests/hermes_cli/test_slack_cli.py:--no-assistantomits all three assistant pieces;--no-assistantpreserves the core messaging surface (Messages tab, slashes, Socket Mode, channel/DM scopes + events);no_assistant=False,--no-assistantsets itTrue.All 7 tests pass;
ruff checkclean on the changed files.