fix(gateway): avoid false Slack API capability denials - #75739
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating the false-negative prompt path. The absolute denial is present on current main at gateway/session.py:584-589, but the proposed fallback currently overcorrects.
Problems
gateway/session.py:583-590promises an authenticated Slack API path whenever_slack_tools_loaded()is false. That detector only checks the native Slack toolset/token and registered Slack-named MCP servers (gateway/session.py:345-388);SessionContextcarries no authenticated skill/CLI/helper capability (gateway/session.py:298-317). A Slack deployment with neither route would receive a false availability claim.- The new test mocks the detector false without configuring a helper, skill, or CLI API surface, then asserts that access remains available (
tests/gateway/test_session.py:175-188). It therefore tests wording rather than the claimed capability condition.
Suggested changes
- Make the helper guidance conditional on a verified, deployment-exposed capability rather than on the absence of native/MCP tools; retain non-promissory behavior otherwise.
- Cover both the no-helper fallback and the verified-helper path, including the existing pinned prompt/key contract in
gateway/run.py:22240-22249.
Automated hermes-sweeper review.
| "Do not promise to perform these actions. The gateway may inline the " | ||
| "current message's Slack block/attachment payload when available, but " | ||
| "you still cannot call Slack APIs yourself." | ||
| "**Platform notes:** You are running inside Slack. Slack API " |
There was a problem hiding this comment.
_slack_tools_loaded() == false does not establish that this deployment has an authenticated skill, CLI, or helper; it only rules out the native/MCP routes checked at gateway/session.py:345-388. Please make this availability claim conditional on an observable capability, and cover the no-helper configuration so it cannot promise an unavailable API.
|
Addressed the review feedback in the updated branch. Affirmative helper guidance is now gated by |
|
CI is green except Python slice 5, which failed in the unrelated |
dac753c to
88f551f
Compare
|
Rebased onto current Rebase status
Local verification on the rebased branch
Why this still needs a maintainer action Could a maintainer rerun the failed job (or the full suite) on the new head? Happy to make any further adjustments. |
88f551f to
292215c
Compare
Summary
Root cause
_slack_tools_loaded()detects dedicated native and MCP model-tool surfaces. A false result proves that those dedicated tools are not loaded, but it does not prove that every deployment-provided Slack route is absent.The previous fallback converted that limited negative signal into an absolute claim:
and:
That instruction can override a governed deployment skill, CLI, plugin, or helper and cause an agent to refuse an authorized operation that the deployment supports.
Fix
The dedicated-tool branch is unchanged.
When no dedicated native or MCP Slack tool is loaded, the platform note now:
Why this revision is narrower
An earlier revision added
slack.authenticated_api_helperconfiguration and multiplex propagation. That boolean could describe intent, but it could not verify the installed helper, authenticated identity, supported methods, or authorization scope.This rewrite removes that configuration surface. It changes only the fallback wording and its regression test.
Test evidence
Current upstream base:
f51aa6a9b5ce514e15f8e337777f522fd5cc6fa2The rewritten patch was also independently reviewed against its sealed fingerprint and stable patch ID. The reviewer confirmed that the positive dedicated-tool branch is byte-identical to upstream and found no security concerns or logic errors.
Scope
Related work