fix(gateway/slack): ephemeral slash-command ack, private notice delivery, format_message fixes#18198
Merged
Merged
Conversation
Slack slash commands (/q, /btw, /stop, /model, etc.) previously showed no user-visible acknowledgement and posted command replies as public channel messages. This diverged from Discord, which uses ephemeral deferred responses for slash commands. Changes: - handle_hermes_command now passes response_type='ephemeral' and a 'Running /cmd…' text to ack(), giving the user immediate 'Only visible to you' feedback when they invoke any native slash command. - _handle_slash_command stashes the Slack response_url from the command payload in a per-channel context dict before dispatching to handle_message. - send() checks for a pending slash context and, when found, POSTs to the response_url with replace_original=true to swap the initial ack with the real command reply (e.g. 'Queued for the next turn.'), keeping it ephemeral. - Stale slash contexts are garbage-collected on lookup (120s TTL). - The response_url POST is non-fatal: if it fails, the user already saw the initial ack, and send() returns success=True. Fixes #18182
Adds platform-level private notice delivery abstraction so operational messages (e.g. sethome prompt) can be sent ephemerally on Slack when configured with `slack.notice_delivery: private`. Changes: - gateway/config.py: _normalize_notice_delivery() + GatewayConfig.get_notice_delivery() with per-platform config bridging - gateway/platforms/base.py: send_private_notice() default implementation (falls through to send()) - gateway/platforms/slack.py: send_private_notice() via chat_postEphemeral - gateway/run.py: _deliver_platform_notice() helper replaces direct adapter.send() for the sethome notice, with private→public fallback - gateway/platforms/slack.py: app_mention handler now forwards to _handle_slack_message (safe due to ts-based dedup) instead of no-op pass, fixing edge-case Slack configs where mentions arrive only as app_mention - gateway/platforms/slack.py format_message: negative lookbehind prevents markdown images (![]()) from becoming broken Slack links; italic regex now requires non-whitespace boundaries so 'a * b * c' stays literal Based on PR #9340 by @probepark.
Required for contributor_audit.py strict mode on the salvaged PR #9340 commit.
…isolation Self-review fixes for the slash ephemeral ack: - Only stash response_url when text starts with '/' (gateway command). Free-form questions via '/hermes <question>' must produce public agent replies visible to the whole channel, not ephemeral. - Use a ContextVar (_slash_user_id) to thread the invoking user's ID from _handle_slash_command through to send(). _pop_slash_context now matches the exact (channel_id, user_id) key when the ContextVar is set, preventing concurrent users on the same channel from stealing each other's ephemeral context. ContextVars propagate to child asyncio.Tasks, so the value survives through handle_message → _process_message_background → _send_with_retry → send(). - Add truncate_message() in _send_slash_ephemeral to prevent silent failures on long responses (response_url has the same ~40k limit). - Log send_private_notice failures at debug level instead of bare except/pass — aids diagnostics without spamming. - Document app_mention dedup dependency on shared event ts. - Add tests: free-form question must NOT stash context, concurrent users on the same channel get isolated contexts, non-slash send() path fallback behavior.
5fac11e to
7b8a54e
Compare
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #18182 and salvages #9340 — comprehensive Slack ephemeral messaging improvements:
/q,/btw,/stop,/model, etc.) and route command replies ephemerally, matching Discord's behavior.chat_postEphemeralwhenslack.notice_delivery: privateis configured.format_messagebug fixes — markdown images no longer produce broken Slack links, and literal asterisks with spaces (a * b * c) are no longer mistakenly italicized.Changes
Commit 1:
fix(gateway/slack): ephemeral ack and routing for slash commandsFixes #18182 — Two gaps combined to produce the bug:
Gap 1 fix — Immediate ephemeral ack:
handle_hermes_commandnow passesresponse_type="ephemeral"and"Running /cmd…"text toack(). Previously the bareawait ack()sent an empty 200 OK, which Slack silently swallowed.Gap 2 fix — Ephemeral reply routing via
response_url:_handle_slash_commandstashes the Slackresponse_urlfrom the command payload in_slash_command_contexts(keyed by(channel_id, user_id)) before dispatching.send()checks for a pending slash context. When found, POSTs toresponse_urlwithreplace_original: trueto swap the ack with the real reply, keeping it ephemeral.Commit 2:
feat(gateway): private notice delivery and Slack format_message fixesSalvaged from PR #9340 by @probepark. Cherry-picked onto current main with original authorship preserved.
gateway/config.py_normalize_notice_delivery()+GatewayConfig.get_notice_delivery()with per-platform config bridginggateway/platforms/base.pysend_private_notice()default implementation (falls through tosend())gateway/platforms/slack.pysend_private_notice()viachat_postEphemeralgateway/run.py_deliver_platform_notice()helper replaces directadapter.send()for the sethome notice, with private→public fallbackgateway/platforms/slack.pyapp_mentionhandler now forwards to_handle_slack_message(safe due to ts-based dedup) instead of no-opgateway/platforms/slack.pyformat_message: negative lookbehind prevents markdown images from becoming broken Slack links; italic regex requires non-whitespace boundariesCommit 3:
chore: add probepark to AUTHOR_MAPCommit 4:
fix(gateway/slack): review fixes — scope ephemeral to commands, user isolationSelf-review caught and fixed:
Critical — Free-form
/hermes <question>routed agent reply ephemeral. The context was stashed unconditionally, so/hermes what's the weatherwould make the full agent response invisible to the channel. Fix: Only stash whentext.startswith("/").Critical — Concurrent users on same channel could steal each other's ephemeral context.
_pop_slash_contextscanned by channel_id only, so User B's response could consume User A'sresponse_url. Fix: Added aContextVar(_slash_user_id) that threads the invoking user's ID from_handle_slash_commandthrough tosend()._pop_slash_contextnow matches the exact(channel_id, user_id)key. ContextVars propagate to childasyncio.Tasks, so the value survives throughhandle_message→_process_message_background→_send_with_retry→send().Medium —
_send_slash_ephemeralskippedtruncate_message(). Long responses could silently fail. Fixed.Warning — Bare
except Exception: passin_deliver_platform_notice. Fixed: Logs at debug level.Docs —
app_mentiondedup dependency on shared event ts. Added comment.Files changed
gateway/platforms/slack.pygateway/config.pygateway/platforms/base.pygateway/run.pytests/gateway/test_slack.pytests/gateway/test_config.pytests/gateway/test_notice_delivery.pyscripts/release.pyTest plan
py_compileverified on all modified files