Skip to content

fix(slack): honor NO_PROXY for Slack transport#12011

Closed
badgerbees wants to merge 1 commit into
NousResearch:mainfrom
badgerbees:fix/slack-no-proxy
Closed

fix(slack): honor NO_PROXY for Slack transport#12011
badgerbees wants to merge 1 commit into
NousResearch:mainfrom
badgerbees:fix/slack-no-proxy

Conversation

@badgerbees

Copy link
Copy Markdown
Contributor

What does this PR do?

This PR fixes Slack proxy behavior in Hermes-agent without changing the rest of the networking stack. Slack traffic now resolves proxy settings locally, only accepts standard HTTP and HTTPS proxies, and disables proxying when NO_PROXY excludes the Slack hosts Hermes actually uses. That keeps the change narrow, preserves existing behavior for other platforms, and avoids introducing extra proxy complexity.

This matters because Slack Socket Mode and API calls are sensitive to proxy configuration. The previous behavior could route Slack through a proxy even when the environment intended Slack to bypass it, or try to use a proxy scheme Slack does not support. The new behavior makes Slack transport predictable across environments and reduces the chance of connection failures caused by proxy configuration drift.

Related Issue

N/A

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • Added Slack-specific proxy resolution that only accepts HTTP and HTTPS proxy URLs.
  • Added NO_PROXY handling so Slack bypasses proxying when the Slack hosts are excluded.
  • Kept the proxy decision local to the Slack adapter so other platforms are not affected.
  • Added regression coverage for unsupported proxy schemes, NO_PROXY bypass behavior, and proxy application during Slack connect.

How to Test

  1. Run the Slack regression slice: pytest -q -n 4 [test_slack.py]
  2. Repeat the Slack connect checks with a proxy configured, then confirm the adapter uses the proxy for Slack WebClient and Socket Mode.
  3. Set NO_PROXY to include slack.com, files.slack.com, or wss-primary.slack.com, rerun the same Slack tests, and confirm the adapter bypasses proxying.
  4. Try an unsupported proxy scheme such as socks5://... and confirm Slack ignores it instead of failing startup.
  5. Send a normal Slack message, a file upload, and a threaded reply to confirm the rest of the adapter still behaves normally after the proxy change.

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: Windows 11

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

For New Skills

  • This skill is broadly useful to most users (if bundled) — see Contributing Guide
  • SKILL.md follows the standard format (frontmatter, trigger conditions, steps, pitfalls)
  • No external dependencies that aren't already available (prefer stdlib, curl, existing Hermes tools)
  • I've tested the skill end-to-end: hermes --toolsets skills -q "Use the X skill to do Y"

Screenshots / Logs

@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 labels Apr 24, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Merged via #16288 — your commit was cherry-picked onto current main with your authorship preserved (dfc773d). Appreciated keeping the proxy resolution local to the Slack adapter rather than touching the global networking stack — made this cleanly scoped to merge.
#16288

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 type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants