Skip to content

fix(slack): exclude reserved Slack commands from native slash manifest#18456

Closed
priveperfumes wants to merge 1 commit into
NousResearch:mainfrom
prive-hn:fix/slack-reserved-command-filter
Closed

fix(slack): exclude reserved Slack commands from native slash manifest#18456
priveperfumes wants to merge 1 commit into
NousResearch:mainfrom
prive-hn:fix/slack-reserved-command-filter

Conversation

@priveperfumes
Copy link
Copy Markdown
Contributor

@priveperfumes priveperfumes commented May 1, 2026

What does this PR do?

When running hermes slack manifest --write, the generated JSON manifest includes /status as a slash command. Slack rejects the manifest because /status is a built-in Slack command that apps cannot register.

This was introduced in #16164 which surfaces all gateway commands as native Slack slashes.

The fix adds a _SLACK_RESERVED_COMMANDS frozenset of all known Slack built-in slash commands and skips them in slack_native_slashes(). Affected commands remain reachable via /hermes <command>.

Related Issue

No existing issue — discovered during app setup.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • hermes_cli/commands.py — Added _SLACK_RESERVED_COMMANDS frozenset (19 built-ins: /status, /me, /join, /away, /dnd, /shrug, /remind, /msg, /feed, /who, /collapse, /expand, /leave, /open, /search, /topic, /mute, /pro, /shortcuts). Added guard in slack_native_slashes() _add() closure to skip reserved names. Updated docstring.
  • tests/hermes_cli/test_commands.py — New test_excludes_slack_reserved_commands. Updated test_includes_canonical_commands (removed /status from expected set). Updated test_telegram_parity to exclude reserved commands from parity check.

How to Test

  1. pytest tests/hermes_cli/test_commands.py::TestSlackNativeSlashes -v — all 9 tests pass
  2. pytest tests/hermes_cli/test_commands.py::TestSlackAppManifest -v — all 4 tests pass
  3. python3 -c "from hermes_cli.commands import slack_native_slashes; names = {n for n,_,_ in slack_native_slashes()}; print('status' in names)" — should print False

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: Ubuntu 24.04 (Linux VM)

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

References

Slack has built-in slash commands (e.g. /status, /me, /join) that apps
cannot register. When running `hermes slack manifest --write`, the
generated manifest included /status, causing Slack to reject the entire
manifest with a reserved-command error.

Add _SLACK_RESERVED_COMMANDS frozenset of all known Slack built-ins and
skip them in slack_native_slashes(). Affected commands remain reachable
via /hermes <command>.

Tests updated:
- New test_excludes_slack_reserved_commands validates no leaks
- test_includes_canonical_commands no longer asserts /status
- test_telegram_parity accounts for expected Slack-only exclusions
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard platform/slack Slack app adapter labels May 1, 2026
@priveperfumes
Copy link
Copy Markdown
Contributor Author

CI Status

The test job has 9 failures, all of which are pre-existing on main (confirmed against run 25215431732):

Test Also fails on main
test_send_available_commands_update ✅ (hardcoded command list outdated)
test_send_typing (Teams) ✅ (async mock)
test_no_yes_flag_still_prompts_in_tty ✅ (update wizard)
test_yes_restores_stash_without_prompting ✅ (update wizard)
test_concurrent_interrupt_cancels_pending ✅ (_Stub missing _tool_guardrails)
test_running_concurrent_worker_sees_is_interrupted ✅ (same)
test_dockerfile_installs_tui_dependencies ✅ (Dockerfile paths)
test_dockerfile_materializes_local_tui_ink_package ✅ (same)
test_session_create_drops_pending_title_on_valueerror ✅ (TUI gateway)

None of these touch hermes_cli/commands.py or the Slack manifest generation. The 13 Slack-specific tests (TestSlackNativeSlashes + TestSlackAppManifest) all pass locally and in CI (they are part of the 18,944 that passed).

@priveperfumes priveperfumes marked this pull request as ready for review May 1, 2026 16:07
@jakeschaeffer
Copy link
Copy Markdown

Ran into. this issue personally while setting up a Slack bot using the auto-generated manifest. Had to change /status name due to a conflict with existing Slack commands not allowing this name.

@kshitijk4poor
Copy link
Copy Markdown
Collaborator

Merged via PR #18553. Your commit was cherry-picked onto current main with your authorship preserved (rebase merge). Thanks for the contribution! 🎉

kshitijk4poor added a commit that referenced this pull request May 1, 2026
Adds email→username mappings for:
- priveperfumes (PR #18456)
- amroessam (PR #17798)
- Hinotoi-agent (PR #9361)
- valda (PR #14932)
donald131 pushed a commit to donald131/hermes-agent that referenced this pull request May 2, 2026
Adds email→username mappings for:
- priveperfumes (PR NousResearch#18456)
- amroessam (PR NousResearch#17798)
- Hinotoi-agent (PR NousResearch#9361)
- valda (PR NousResearch#14932)
nickdlkk pushed a commit to nickdlkk/hermes-agent that referenced this pull request May 11, 2026
Adds email→username mappings for:
- priveperfumes (PR NousResearch#18456)
- amroessam (PR NousResearch#17798)
- Hinotoi-agent (PR NousResearch#9361)
- valda (PR NousResearch#14932)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists platform/slack Slack app adapter type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants