Skip to content

fix(slack): handle leading-space text commands - #56718

Closed
nu476 wants to merge 1 commit into
NousResearch:mainfrom
nu476:nu/slack-leading-space-commands-20260702-100311
Closed

fix(slack): handle leading-space text commands#56718
nu476 wants to merge 1 commit into
NousResearch:mainfrom
nu476:nu/slack-leading-space-commands-20260702-100311

Conversation

@nu476

@nu476 nu476 commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • treat text like /stop as a gateway command after trimming leading Slack-avoidance whitespace
  • avoid prepending thread context to command text in Slack thread replies
  • add e2e coverage for leading-space stop commands

Test

  • uv run pytest tests/e2e/test_platform_commands.py::TestSlashCommands::test_leading_space_stop_is_still_a_command tests/e2e/test_platform_commands.py::TestSlashCommands::test_stop_when_no_agent_running -q

@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery comp/plugins Plugin system and bundled plugins platform/slack Slack app adapter P2 Medium — degraded but workaround exists sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Jul 2, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: leading-space slash-command cluster. This PR fixes it at the shared gateway/platforms/base.py MessageEvent layer (lstrip before command detection) plus the Slack adapter, whereas #21070 and #32210 fix the same class of bug scoped to Mattermost only. #25355 added !cmd as a Slack thread alternate prefix. Flagging for a maintainer to pick between the shared-base approach here vs. the per-platform patches.

@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 the focused Slack command fix. The premise is confirmed on current main: plugins/platforms/slack/adapter.py:3410 recognizes only text beginning with /, while first thread replies fetch and prepend context at plugins/platforms/slack/adapter.py:3391-3406 before that classification.

Problems

  • tests/e2e/test_platform_commands.py:60 does not exercise the changed Slack inbound path. Its helper constructs a MessageEvent directly (tests/e2e/conftest.py:273-281) and invokes adapter.handle_message(), bypassing SlackAdapter._handle_slack_message() where this PR adds the normalization and thread-context guard.

Suggested changes

  • Add a raw Slack-event test in tests/gateway/test_slack.py for " /stop" through _handle_slack_message(), asserting a /stop MessageEvent with MessageType.COMMAND.
  • Add a threaded case asserting _fetch_thread_context() is not called for that command.
  • The Mattermost precedent is adapter-local normalization (plugins/platforms/mattermost/adapter.py:881); preserve or intentionally justify the broader shared-base behavior during salvage.

Automated hermes-sweeper review.

assert "no" in response_lower or "stop" in response_lower or "not running" in response_lower

@pytest.mark.asyncio
async def test_leading_space_stop_is_still_a_command(self, adapter, platform):

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.

This test creates a MessageEvent directly through send_and_capture(), so it bypasses SlackAdapter._handle_slack_message() entirely. Please add a Slack-adapter test with a raw {"text": " /stop"} event, including a thread case that verifies _fetch_thread_context() is not called.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jul 15, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Merged via #69479 — your commit was cherry-picked/reapplied onto current main with your authorship preserved in git history: your leading-space command recognition was cherry-picked directly.

Thanks for the contribution!

@teknium1 teknium1 closed this Jul 23, 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 P2 Medium — degraded but workaround exists platform/slack Slack app adapter sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit 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 type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants