Skip to content

fix(slack): surface attachment access diagnostics#7245

Closed
xinbenlv wants to merge 1 commit into
NousResearch:mainfrom
xinbenlv:fix/slack-download-diagnostics
Closed

fix(slack): surface attachment access diagnostics#7245
xinbenlv wants to merge 1 commit into
NousResearch:mainfrom
xinbenlv:fix/slack-download-diagnostics

Conversation

@xinbenlv

Copy link
Copy Markdown
Contributor

Summary

  • surface actionable Slack attachment diagnostics for scope/auth/permission/access failures instead of hiding them behind generic cache/download errors
  • probe file access with files.info when possible and translate Slack API responses like missing_scope, invalid_auth, and permission-denied variants into user-facing attachment notices
  • preserve those diagnostics in the message text so the agent can tell the user what to fix, while still logging the detail server-side
  • update Slack setup docs and interactive config prompts to include files:read

Test Plan

  • /Users/peteradams/.hermes/hermes-agent/venv/bin/python -m pytest tests/gateway/test_media_download_retry.py tests/gateway/test_slack.py -q -o addopts=''

Related

Contributed back from https://github.com/xinbenlv/zn-hermes-agent, which is being used and tested by https://github.com/xinbenlv.

@teknium1

Copy link
Copy Markdown
Contributor

Merged via #16206 — your commit was cherry-picked onto current main with your authorship preserved (67cb62f). Thanks for the thorough diagnostic helpers, especially the mapping from Slack API error codes (missing_scope, invalid_auth, file_not_found, access_denied, etc.) to actionable user-facing text with scope hints and reinstall guidance.

One adjustment: dropped _probe_slack_file_access_issue (the proactive pre-download files.info call). Two reasons — it added one extra Slack API round-trip per attachment even on healthy ones, and it overlapped with the files.info call that PR #11111 added for Slack Connect stubs. The post-failure translation via _describe_slack_download_failure + wiring _describe_slack_api_error into #11111's existing files.info call covers the same user-facing diagnostic surface without the per-message tax.
#16206

@teknium1 teknium1 closed this Apr 26, 2026
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists platform/slack Slack app adapter comp/gateway Gateway runner, session dispatch, delivery labels Apr 26, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Superseded by #16206 — salvage of this PR merged with adjustments (dropped proactive files.info probe, kept post-failure diagnostics). Same Slack attachment diagnostic surfacing.

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