Skip to content

fix(slack): progress/status card lifecycle — edit in place, always clear, survive transients - #70189

Merged
teknium1 merged 8 commits into
mainfrom
slack/c6-progress-cards
Jul 23, 2026
Merged

fix(slack): progress/status card lifecycle — edit in place, always clear, survive transients#70189
teknium1 merged 8 commits into
mainfrom
slack/c6-progress-cards

Conversation

@teknium1

Copy link
Copy Markdown
Contributor

Summary

Slack progress/status messages no longer hang, duplicate, or outlive the reply: status bubbles edit in place, finalization always clears status (including error paths), transient network failures don't kill progress edits, and stale progress messages are cleaned up.

Fixes #36780, #24117.

Changes

Credits

Salvaged with authorship preserved: #68378 (@trac3r00), #64267 (@2001Y), #57196 (@Trantor-develops), #18859, #17184.
Flag: #68552 (default tool progress off) NOT included — behavior-default change needing explicit maintainer sign-off; recommend separate decision.

Validation

Check Result
tests/gateway/ -q -k slack 790 passed, 0 failed (re-verified post-rebase)
Exception-path cleanup regression tests green

Infographic

slack-status-cards

@teknium1 teknium1 added the platform/slack Slack app adapter label Jul 23, 2026
@github-actions

github-actions Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

૮ >ﻌ< ა ci review

ran on be14828

ℹ️ Info

Desktop E2E visual evidence · View test artifacts · View job

1 visual diff.

inline evidence upload failed.

Failed to upload diff-665a0833239e-onboarding-overlay-diff.png with gh image (exit code 1): Error uploading /home/runner/work/_temp/e2e-evidence/diff-665a0833239e-onboarding-overlay-diff.png: step 0 (get upload token): uploadToken not found on repo page — do you have write access to NousResearch/hermes-agent? (or, if NousResearch enforces SAML SSO, authorize at https://github.com/orgs/NousResearch/sso)

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery comp/plugins Plugin system and bundled plugins sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Jul 23, 2026
trac3r00 and others added 8 commits July 23, 2026 09:49
Progress/status callbacks (context-pressure, compression retries,
model fallback) route through _send_or_update_status_coro, which
edits the previous bubble for the same status_key when the adapter
implements send_or_update_status — but only Telegram did. On Slack
every status event posted a fresh thread message, so a compression
retry loop spammed a dozen out-of-order bubbles into the thread
('Context too large 1/3... 2/3... 3/3', fallback switches, etc.).

Implement send_or_update_status on the Slack adapter following the
Telegram pattern (#30045): first call posts and caches the message ts
per (channel, thread, status_key); subsequent calls edit that message
via chat.update. Edit failure drops the cached ts and falls back to a
fresh send. Cache is FIFO-bounded.
When reply_in_thread=false, top-level channel events carry their own
message ts as metadata.thread_id for session keying. Calling
assistant.threads.setStatus on that ts activated a Slack assistant
thread ('is thinking...') before the actual response was sent, and the
flat reply then never cleared it.

send_typing now routes through the same _resolve_thread_ts synthetic-
thread guard as message sending, and the gateway threads message_id
through progress/status metadata so the adapter can distinguish real
threads from synthetic top-level session keys.

Reapplied from #18859-sibling PR #17184 by @dorukardahan (both commits:
fix + progress-metadata test) onto current main via 3-way apply — the
original patched gateway/platforms/slack.py, moved to
plugins/platforms/slack/adapter.py in the plugin migration.
The Slack adapter honours platforms.slack.extra.reply_in_thread=false
in _resolve_thread_ts, but the Gateway's progress-message path forced
event_message_id as the thread_id for Slack regardless. The first
progress message ('terminal: …', 'Processing…') created a thread that
all subsequent edits and the final answer inherited, defeating the
user's reply_in_thread=false setting.

Check the live Slack adapter's reply_in_thread flag before applying the
event_message_id fallback, and treat a synthetic source.thread_id (==
the event's own message ts, used only for session keying) as 'no
thread' so progress messages stay at the channel/DM top level.

Folds both #18859 commits (reply_in_thread gate + synthetic thread_id
drop) into main's extracted _resolve_progress_thread_id helper — the
original patched the pre-refactor inline block; the gate now composes
as a keyword argument so Mattermost/other platforms keep the default
fallback behavior.
Widening for #24117 ('is thinking...' stuck after the response was
sent): the stuck-thinking class is a missing-cleanup-on-error-path bug.
send()'s status clear was gated on thread_ts already being resolved and
on reaching the normal post-message path, which left the Slack
Assistant status visible when a turn ended through any sibling exit:

- exception BEFORE _resolve_thread_ts (slash-context handling,
  formatting, DM resolution) — the 'if thread_ts: stop_typing' clear
  never ran
- empty/whitespace-only final response (no_text guard early-return)
- ephemeral slash replies (Slack only auto-clears assistant status on
  real thread replies; ephemerals never count)

Add _clear_thread_status_quietly() — a best-effort stop_typing wrapper
that never masks the caller's SendResult — and wire it at every send()
exit plus the finalize paths of edit_message. stop_typing already
handles the untracked-thread fallback (clearing an unset status is a
no-op on Slack's side), so this is pure coverage widening.

Also add the #18859 unit tests for _resolve_progress_thread_id's new
reply_in_thread gate (synthetic-thread drop, real-thread keep, event-id
fallback suppression, default unchanged).

A/B: 3 of the 4 new status-clear tests fail with the adapter widening
reverted and pass with it applied; the fourth pins the existing
cleanup-must-not-mask-result contract.
- rt.cms012@gmail.com -> trac3r00 (#68378; commit authored as
  'Minseo-Choi' — trac3r00's display name, same account)
- 15167896+2001Y@users.noreply.github.com -> 2001Y (#64267)
- hello@jeromeiveson.com -> Trantor-develops (#57196)
- boumagent@gmail.com -> patp (#18859)
- dorukardahan@hotmail.com -> dorukardahan (#17184) was already mapped.
… tests

CI shards run without the slack extras; the two #64267 tests import the
real SDK (SlackApiError, the lazy-rebind path) and errored with
ModuleNotFoundError. Skip on bare environments — classification coverage
for stdlib exception types (OSError/TimeoutError/cert errors) still runs
everywhere.
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 comp/plugins Plugin system and bundled plugins P2 Medium — degraded but workaround exists platform/slack Slack app adapter sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Slack progress/status cards can hang or remain stale during finalization

7 participants