Repository navigation
Conversation
dhigginbotham
left a comment
There was a problem hiding this comment.
Reviewed by Daemon. Requesting changes: the new Slack status clearing logic does not appear to cover the main /stop fast paths.
Blocking findings:
-
gateway/run.py:7036—/stopwhile an agent is actively running is intercepted before normal command dispatch. That path calls_interrupt_and_clear_session(...)and returns anEphemeralReply, bypassing_handle_stop_command()where the new_clear_typing_indicator()helper lives. This can still leave Slack assistant/thread status stuck for the primary active-agent stop scenario. -
gateway/run.py:7268— the pending-agent/stopfast path also releases state and returns directly, bypassing the new clearing logic. Ifsend_typing()has already set Slack status during startup/pending state, this can also leave status uncleared.
Targeted tests run by reviewer: python -m pytest tests/gateway/test_slack.py::TestSendTyping tests/gateway/test_slack_stop_command.py -q => 12 passed.
Suggested fix: call the Slack typing/status clear helper (or equivalent adapter stop_typing) in the active-running and pending-agent /stop fast paths as well, and add regression tests for both paths.
teknium1
left a comment
There was a problem hiding this comment.
Thanks for identifying the lost-map case. The issue is still present on current main, but the proposed placement does not reach the main /stop fast paths.
Problems
- Current main intercepts active-agent
/stopatgateway/run.py:9257and pending-agent/stopatgateway/run.py:9519, bypassing the PR's_handle_stop_command()helper. The active path reachesBasePlatformAdapter.interrupt_session_activity()which callsstop_typing(chat_id)without metadata (gateway/platforms/base.py:3931-3939); that cannot use the proposed fallback when Slack's map is absent. - The Slack adapter has moved to
plugins/platforms/slack/adapter.py:1544;/stopcommand handling is now ingateway/slash_commands.py:1054. This needs a port, not a clean application of the current diff.
Suggested changes
- Port the metadata fallback to
plugins/platforms/slack/adapter.py, then route metadata-aware clearing through normal, active, and pending/stoppaths. - Add regressions for each of those paths with
_active_status_threadsabsent.
Automated hermes-sweeper review.
| source = event.source | ||
| session_entry = self.session_store.get_or_create_session(source) | ||
| session_key = session_entry.session_key | ||
| adapter = self.adapters.get(source.platform) |
There was a problem hiding this comment.
This helper is only reached through normal command dispatch. /stop while an agent is active or pending is intercepted earlier and returns without reaching this method, so the primary stuck-status paths remain uncovered. Please route the same metadata-aware clear through those fast paths and add regressions.
…tadata Salvaged from #32340 by @LeonSGP43, adapted to the workspace-scoped status tracking that landed in #63709: - /stop with no running agent now best-effort clears the platform status indicator, so a phantom 'is thinking...' left by a gateway restart or a turn that died without a final send can always be dismissed (#32295). - SlackAdapter.stop_typing clears an untracked thread when the caller names it explicitly in metadata — clearing an unset status is a harmless no-op on Slack's side. The fallback is skipped when multiple Slack Connect workspaces track the same channel+thread and no team_id is given, preserving #63709's cross-workspace safety guarantee.
|
Merged via #64621 — your commit was adapted onto current main with your authorship preserved in git log ( The core of your fix survived intact: Thanks @LeonSGP43! |
…tadata Salvaged from NousResearch#32340 by @LeonSGP43, adapted to the workspace-scoped status tracking that landed in NousResearch#63709: - /stop with no running agent now best-effort clears the platform status indicator, so a phantom 'is thinking...' left by a gateway restart or a turn that died without a final send can always be dismissed (NousResearch#32295). - SlackAdapter.stop_typing clears an untracked thread when the caller names it explicitly in metadata — clearing an unset status is a harmless no-op on Slack's side. The fallback is skipped when multiple Slack Connect workspaces track the same channel+thread and no team_id is given, preserving NousResearch#63709's cross-workspace safety guarantee.
…tadata Salvaged from NousResearch#32340 by @LeonSGP43, adapted to the workspace-scoped status tracking that landed in NousResearch#63709: - /stop with no running agent now best-effort clears the platform status indicator, so a phantom 'is thinking...' left by a gateway restart or a turn that died without a final send can always be dismissed (NousResearch#32295). - SlackAdapter.stop_typing clears an untracked thread when the caller names it explicitly in metadata — clearing an unset status is a harmless no-op on Slack's side. The fallback is skipped when multiple Slack Connect workspaces track the same channel+thread and no team_id is given, preserving NousResearch#63709's cross-workspace safety guarantee.
…tadata Salvaged from NousResearch#32340 by @LeonSGP43, adapted to the workspace-scoped status tracking that landed in NousResearch#63709: - /stop with no running agent now best-effort clears the platform status indicator, so a phantom 'is thinking...' left by a gateway restart or a turn that died without a final send can always be dismissed (NousResearch#32295). - SlackAdapter.stop_typing clears an untracked thread when the caller names it explicitly in metadata — clearing an unset status is a harmless no-op on Slack's side. The fallback is skipped when multiple Slack Connect workspaces track the same channel+thread and no team_id is given, preserving NousResearch#63709's cross-workspace safety guarantee.
…tadata Salvaged from NousResearch#32340 by @LeonSGP43, adapted to the workspace-scoped status tracking that landed in NousResearch#63709: - /stop with no running agent now best-effort clears the platform status indicator, so a phantom 'is thinking...' left by a gateway restart or a turn that died without a final send can always be dismissed (NousResearch#32295). - SlackAdapter.stop_typing clears an untracked thread when the caller names it explicitly in metadata — clearing an unset status is a harmless no-op on Slack's side. The fallback is skipped when multiple Slack Connect workspaces track the same channel+thread and no team_id is given, preserving NousResearch#63709's cross-workspace safety guarantee.
Fixes #32295
Summary
/stopeven when no running agent is registeredSlackAdapter.stop_typing()clear a thread from explicit metadata when the in-memorychat_id -> thread_tsmap is gone/stopno-active pathTesting
uv run --frozen pytest -q -o addopts='' tests/gateway/test_slack.py -k 'stop_typing'uv run --frozen pytest -q -o addopts='' tests/gateway/test_slack_stop_command.pygit diff --checkuv run --frozen ruff check gateway/platforms/slack.py gateway/run.py tests/gateway/test_slack.py tests/gateway/test_slack_stop_command.py