Skip to content

fix: quiet Slack control flow - #36670

Closed
jakelongvu-bot wants to merge 1 commit into
NousResearch:mainfrom
jakelongvu-bot:codex/slack-action-auth-status-noise
Closed

fix: quiet Slack control flow#36670
jakelongvu-bot wants to merge 1 commit into
NousResearch:mainfrom
jakelongvu-bot:codex/slack-action-auth-status-noise

Conversation

@jakelongvu-bot

Copy link
Copy Markdown
Contributor

Summary

  • authorize Slack approval/slash-confirm button clicks through the same SLACK_ALLOWED_USERS, GATEWAY_ALLOWED_USERS, and paired-user store used for normal messages
  • attach the GatewayRunner to the Slack adapter so paired Slack users can approve actions from buttons
  • suppress Slack busy/lifecycle/status callback messages so shared channels get the final assistant reply instead of extra gateway chatter

Root Cause

Slack button interactions bypass the normal message authorization path in gateway/run.py. That meant a user who was paired/authorized for Slack messages could still fail approval-button flows, and the gateway could post implementation-status noise into shared Slack channels.

Validation

  • remote clean verification using staging venv:
    • python -m py_compile gateway/platforms/slack.py gateway/run.py tests/gateway/test_slack_approval_buttons.py tests/gateway/test_busy_session_ack.py
    • python -m pytest tests/gateway/test_slack_approval_buttons.py tests/gateway/test_busy_session_ack.py -q (46 passed)
  • git diff --check

@jakelongvu-bot

Copy link
Copy Markdown
Contributor Author

Vucar staging is already patched and restarted (Hermes gateway PID 1231745). Targeted staging tests passed: tests/gateway/test_slack_approval_buttons.py tests/gateway/test_busy_session_ack.py -q -> 46 passed. @thomasdongg please review/coordinate deploy with Charles/Viet Thinh.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery platform/slack Slack app adapter type/security Security vulnerability or hardening labels Jun 1, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the Slack security and delivery-focused work. The approval-button authorization issue is already resolved on current main by commit 410cb743bf6f3a7dd0b581f927bff719338a06fa (merged through #41226): plugins/platforms/slack/adapter.py:3406-3453 delegates interactive authorization to the canonical gateway auth path, and both protected action handlers gate before state mutation (:3466-3475, :3585-3594).

Problems

  • The PR's Slack adapter edits target gateway/platforms/slack.py, which has since moved to plugins/platforms/slack/adapter.py. Current wiring uses adapter.set_message_handler(self._handle_message) at gateway/run.py:7062-7069; the plugin derives the runner from that bound handler at plugins/platforms/slack/adapter.py:3418-3431.
  • The quiet-control-flow objective remains distinct: current main still enables busy acknowledgements by default (gateway/run.py:5510-5513) and schedules non-filtered status callbacks (gateway/run.py:17933-17953). The generic filter suppresses only matching transient status text (gateway/run.py:440-457).

Suggested changes

  • Re-scope salvage to the remaining Slack quiet-flow behavior and add current-head regression coverage for the selected busy/status policy.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Merged via #70192 — your commit was cherry-picked/reapplied onto current main with your authorship preserved in git history: your control-flow quieting was salvaged.

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 P2 Medium — degraded but workaround exists platform/slack Slack app adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants