Skip to content

fix(slack): resolve file events with cached workspace team - #30456

Closed
robzolkos wants to merge 1 commit into
NousResearch:mainfrom
robzolkos:fix/slack-file-events-team-cache
Closed

fix(slack): resolve file events with cached workspace team#30456
robzolkos wants to merge 1 commit into
NousResearch:mainfrom
robzolkos:fix/slack-file-events-team-cache

Conversation

@robzolkos

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes Slack file handling for multi-workspace gateway setups where Slack file-upload events omit team / team_id.

When Hermes has multiple Slack workspaces configured, file downloads need the correct workspace bot token. Regular messages populate the channel-to-team cache, but some file events arrive without workspace metadata. This PR falls back to the cached channel workspace before downloading attachments, so Hermes uses the correct Slack client/token.

Related Issue

No existing issue. This PR adds a regression test for the reported multi-workspace Slack file-event behavior.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✅ Tests (adding or improving test coverage)

Changes Made

  • Updated gateway/platforms/slack.py to resolve missing Slack team_id from the cached channel workspace mapping.
  • Added regression coverage in tests/gateway/test_slack.py for file events that omit team / team_id.

How to Test

  1. Run the Slack-related test suite:

    scripts/run_tests.sh tests/gateway/test_slack.py tests/gateway/test_slack_approval_buttons.py tests/gateway/test_slack_channel_skills.py tests/gateway/test_slack_mention.py tests/hermes_cli/test_slack_cli.py
  2. Confirm all Slack tests pass:

    279 tests passed, 0 failed
    

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 the relevant test suite and all targeted tests pass
  • I've added tests for my changes
  • I've tested on my platform: Linux

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A

Screenshots / Logs

N/A

@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 May 22, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused multi-workspace regression fix. The premise still holds on current main: plugins/platforms/slack/adapter.py:2749-2757 resolves team_id from the event or assistant metadata but not _channel_team, while attachment downloads use that value (for example, :3096-3098) and fall back to the primary token when it is empty (:4105-4113).

Problems

  • The production hunk targets the former inline adapter path, gateway/platforms/slack.py. Commit 5600105478ffde29d7566b45421b100eaa29c4ef moved it to plugins/platforms/slack/adapter.py, so this needs a mechanical relocation before it can land.

Suggested changes

  • Apply the cached-channel fallback immediately after the current team_id extraction at plugins/platforms/slack/adapter.py:2749-2753, before the existing cache-write at :2755-2757.
  • Keep the regression test; tests/gateway/test_slack.py:67-71 already imports the plugin adapter. The separate file_shared fallback already consults the channel cache through _get_client(channel_id) at plugins/platforms/slack/adapter.py:2525-2529.

Automated hermes-sweeper review.

@teknium1 teknium1 added 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 sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Merged via #69482 — your commit was cherry-picked/reapplied onto current main with your authorship preserved in git history: your cached workspace-team file-event resolution was re-targeted onto the plugin adapter with your authorship.

Thanks for the contribution!

@teknium1 teknium1 closed this Jul 23, 2026
teknium1 pushed a commit that referenced this pull request Jul 23, 2026
…dded team id

Salvaged from #59742 (downloads half only). Main already resolves the
event-level team id from the channel→workspace cache before downloads
(the file-EVENT half was covered by #30456), but when neither the event
nor the cache knows the workspace, both download helpers silently fell
back to the PRIMARY workspace token — Slack then returns an HTML login
page instead of file bytes for any non-primary workspace file.

Slack private file URLs embed the owning workspace id
(files-pri/<TEAM_ID>-<FILE_ID>/...), so _resolve_download_token now
prefers: explicit team_id -> URL-embedded team id -> primary token.
Both _download_slack_file and _download_slack_file_bytes route through
it.

Deferred from #59742 (out of this correctness cluster's scope): the
channel→team disk persistence, the pre-send conversations.info probe
loop, and the thread-file ingestion feature.

Reapplied from #59742 by @benjamin2026-dot onto the current adapter
(original targeted the pre-#30456 download sites).
teknium1 pushed a commit that referenced this pull request Jul 23, 2026
…dded team id

Salvaged from #59742 (downloads half only). Main already resolves the
event-level team id from the channel→workspace cache before downloads
(the file-EVENT half was covered by #30456), but when neither the event
nor the cache knows the workspace, both download helpers silently fell
back to the PRIMARY workspace token — Slack then returns an HTML login
page instead of file bytes for any non-primary workspace file.

Slack private file URLs embed the owning workspace id
(files-pri/<TEAM_ID>-<FILE_ID>/...), so _resolve_download_token now
prefers: explicit team_id -> URL-embedded team id -> primary token.
Both _download_slack_file and _download_slack_file_bytes route through
it.

Deferred from #59742 (out of this correctness cluster's scope): the
channel→team disk persistence, the pre-send conversations.info probe
loop, and the thread-file ingestion feature.

Reapplied from #59742 by @benjamin2026-dot onto the current adapter
(original targeted the pre-#30456 download sites).
teknium1 pushed a commit that referenced this pull request Jul 23, 2026
…dded team id

Salvaged from #59742 (downloads half only). Main already resolves the
event-level team id from the channel→workspace cache before downloads
(the file-EVENT half was covered by #30456), but when neither the event
nor the cache knows the workspace, both download helpers silently fell
back to the PRIMARY workspace token — Slack then returns an HTML login
page instead of file bytes for any non-primary workspace file.

Slack private file URLs embed the owning workspace id
(files-pri/<TEAM_ID>-<FILE_ID>/...), so _resolve_download_token now
prefers: explicit team_id -> URL-embedded team id -> primary token.
Both _download_slack_file and _download_slack_file_bytes route through
it.

Deferred from #59742 (out of this correctness cluster's scope): the
channel→team disk persistence, the pre-send conversations.info probe
loop, and the thread-file ingestion feature.

Reapplied from #59742 by @benjamin2026-dot onto the current adapter
(original targeted the pre-#30456 download sites).
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
…dded team id

Salvaged from NousResearch#59742 (downloads half only). Main already resolves the
event-level team id from the channel→workspace cache before downloads
(the file-EVENT half was covered by NousResearch#30456), but when neither the event
nor the cache knows the workspace, both download helpers silently fell
back to the PRIMARY workspace token — Slack then returns an HTML login
page instead of file bytes for any non-primary workspace file.

Slack private file URLs embed the owning workspace id
(files-pri/<TEAM_ID>-<FILE_ID>/...), so _resolve_download_token now
prefers: explicit team_id -> URL-embedded team id -> primary token.
Both _download_slack_file and _download_slack_file_bytes route through
it.

Deferred from NousResearch#59742 (out of this correctness cluster's scope): the
channel→team disk persistence, the pre-send conversations.info probe
loop, and the thread-file ingestion feature.

Reapplied from NousResearch#59742 by @benjamin2026-dot onto the current adapter
(original targeted the pre-NousResearch#30456 download sites).
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants