Skip to content

fix: make Slack platform capability note tool-aware - #62135

Closed
tgasgarth-coatue wants to merge 1 commit into
NousResearch:mainfrom
tgasgarth-coatue:fix/slack-platform-capability-note
Closed

fix: make Slack platform capability note tool-aware#62135
tgasgarth-coatue wants to merge 1 commit into
NousResearch:mainfrom
tgasgarth-coatue:fix/slack-platform-capability-note

Conversation

@tgasgarth-coatue

Copy link
Copy Markdown

What changed

  • replace the absolute Slack API denial with capability-aware guidance
  • tell agents to inspect loaded tools, plugins, and bounded helpers before claiming read access is unavailable
  • preserve the guard against unsupported write/admin actions

Why

Profiles can provide bounded read-only Slack helpers even when Hermes has no native Slack tool. The previous absolute note caused an agent to incorrectly refuse a channel-history summary despite a working helper.

Verification

  • direct build_session_context_prompt runtime assertion passed
  • py_compile passed for changed Python files
  • git diff --check passed
  • canonical pytest runner unavailable in this air-gapped workspace because no test venv/pytest cache is installed

@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery platform/slack Slack app adapter duplicate This issue or pull request already exists P3 Low — cosmetic, nice to have labels Jul 10, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #6538 — the earliest open PR making the Slack platform note in build_session_context_prompt capability-aware for the same bug (#6533). #6545 is a second open sibling with the same approach. Marking this as a duplicate of the earliest open PR; a maintainer can pick whichever is cleanest to merge.

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Looks good! No obvious issues found.


Reviewed by Hermes Agent

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for targeting the real Slack prompt/runtime mismatch.

Problems

  • The replacement at gateway/session.py:499 is selected unconditionally. build_session_context_prompt() currently has no loaded-tool argument (gateway/session.py:380-384), and the gateway supplies only context and redact_pii (gateway/run.py:10866). As a result, a Slack session without scoped tools gets the same relaxed note as a session with Slack MCP tools. That misses #6533's required split: retain the conservative note when no scoped tool is available, and emit a capability-aware note only when one is.

Suggested changes

  • Thread a stable, session-selected Slack capability signal into the prompt builder, analogous to the existing Discord capability gate at gateway/session.py:343-365, and cover both tool-present and tool-absent paths in tests/gateway/test_session.py.

Automated hermes-sweeper review.

Comment thread gateway/session.py
@@ -497,11 +497,15 @@ def build_session_context_prompt(
lines.append("")
lines.append(
"**Platform notes:** You are running inside Slack. "

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This note is emitted for every Slack session, regardless of whether a scoped Slack tool is actually exposed. Please select it from an explicit, stable loaded-tool capability signal so no-tool sessions retain the conservative disclaimer required by #6533.

assert "Slack" in prompt
assert "cannot search" in prompt.lower()
assert "pin" in prompt.lower()
assert "does not automatically expose a native slack api tool" in prompt.lower()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test covers only the single generic note. Add separate tool-present and tool-absent cases so the regression contract verifies that no-tool sessions remain conservative while scoped Slack-tool sessions are acknowledged.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 11, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Closing as superseded by #70194 (merged): covered by the derivation approach landed via #70194.

Thanks for the work — it's credited in #70194's summary.

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 duplicate This issue or pull request already exists P3 Low — cosmetic, nice to have platform/slack Slack app adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants