feat(gateway): Slack-native "Thinking Steps" task cards for tool progress (opt-in) - #59010
feat(gateway): Slack-native "Thinking Steps" task cards for tool progress (opt-in)#59010menhguin wants to merge 25 commits into
Conversation
6400b83 to
beb0cfb
Compare
|
can't wait for this to merge! |
|
that's awesome! |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the detailed Slack API investigation. The feature is still absent on current main: gateway progress is still queued through the generic sender (gateway/run.py:17730, gateway/run.py:17812).
Problems
gateway/run.py:18227only assignsagent.reasoning_callbackfor native-card turns. Cached agents are reused, but current main's per-turn reset does not clear callbacks (gateway/run.py:16596-16618). A later turn with the toggle disabled retains the prior closure and can append reasoning to the stopped prior stream.- The PR changes only production files (
gh pr diff 59010 --name-only); no tests cover the new Slack stream lifecycle or callback wiring.
Suggested changes
- Clear
agent.reasoning_callbackon every non-native turn, and add a cached-agent toggle regression test. - Add fake-client gateway tests for enablement/fallback, ordered completion, rollover/replay, and subagent streams.
Automated hermes-sweeper review.
| # are active for this turn — reasoning_callback is otherwise | ||
| # unused by the gateway, so this is strictly additive. | ||
| if _slack_native_cards and _slack_task_stream is not None: | ||
| agent.reasoning_callback = _slack_reasoning_event |
There was a problem hiding this comment.
Please assign agent.reasoning_callback on every turn, including None when native cards are disabled. Cached agents are reused and current _init_cached_agent_for_turn() does not clear callbacks, so toggling this feature off leaves the previous stopped stream closure active.
a2a8382 to
328dc2f
Compare
…ask-stream tests Review items for NousResearch#59010: 1. reasoning_callback is now assigned on EVERY turn in the per-message callback block — the closure when native cards are active, explicit None otherwise — matching the reset pattern of every other callback in that block. Cached agents are reused across turns and _init_cached_agent_for_turn() does not clear callbacks, so the old conditional-only wiring left the previous turn's closure active when the toggle was flipped off (or on a non-Slack turn hitting the same cached agent): reasoning deltas would append to a stopped, stale task stream. 2. tests/gateway/test_slack_task_stream.py (13 tests): - AST invariant pinning the every-turn assignment shape (None branch required; feature-gated if-with-no-else rejected) + a behavioral cached-agent toggle regression test (cards ON turn, then OFF turn on the same agent — stale closure must not fire). - Fake-client SlackTaskStream lifecycle: enablement/fallback (startStream failure disables cards, no retry storm, later events no-op; non-recoverable append error disables for rest of turn), ordered completion (per-card in_progress→complete transitions, timeline order, stop() footer stats, failed tools render complete + '✗ failed' not Slack 'error'), rollover (reactive on message_not_in_streaming_state with in-progress replay onto the fresh stream, proactive on age, runaway guard), subagent streams (numbered lifecycle cards, parallel independence, failure marking), and reasoning-card buffering (pre-tool thinking buffered, flushed by first task_started, thought→tool order). Swept the rest of the per-message block for the same shape: every other agent callback there already assigns unconditionally (with else-None fallbacks where gated). CLI and TUI-dashboard surfaces assign their reasoning callback unconditionally at construction/toggle time, so no equivalent staleness exists there. Co-authored-by: Minh Nguyen <menhguin@users.noreply.github.com>
|
Both points addressed (commits 2d63702 + 328dc2f), plus a sweep for the same shape elsewhere:
Sweep: audited every Also rebased onto #59009's reviewed lineage so both PRs carry identical dedup code. |
Builds on the salvaged typing_status_text plumbing (PR #62007): instead of a static 'is thinking...', Slack's assistant status line now updates live as the agent works — 'is running pytest tests/…', 'is reading docs/api.md…' — and reverts to the static text between tool calls. Mechanics: - agent/display.py: build_status_phrase() derives a <=49-char present- tense phrase from the existing _TOOL_VERBS table (+ 'is using <name>' for plugin/MCP tools; None for _thinking). - base adapter: supports_status_text capability flag + set_status_text() per-chat store, cleared when the typing loop winds down. - Slack adapter: send_typing() renders the live phrase when set, falling back to typing_status_text then 'is thinking...'. - gateway/run.py: progress_callback stashes the phrase on tool.started and clears on tool.completed. Rendering rides the existing _keep_typing refresh cadence — zero additional Slack API calls, no rate-limit exposure. Works with tool_progress: off (Slack default); the callback is now armed whenever the adapter supports status text. - display.live_status config (full|verb|off, default full): 'verb' hides argument previews for shared/customer-facing channels. Also fixes a latent crash in the cherry-picked from_dict: malformed non-dict 'extra' sections broke typing_status_text resolution (uses the already-coerced extra dict). Design notes: status text is a side-effect display channel only — never enters the transcript, no prompt-cache impact. Lifecycle guarantees from the stuck-status fix family are preserved (per-thread tracking, clear-on-finish via existing stop_typing paths). Related: #45109 (closed; same direction via lifecycle states), #59010/#51363 (native task cards — complementary, larger scope).
Renders tool-call progress on Slack as native task cards (chat.startStream/appendStream/stopStream with task_update chunks) instead of markdown progress bubbles. Opt-in via display.platforms.slack.tool_progress_native; strictly additive. - gateway/slack_task_stream.py (new): SlackTaskStream lifecycle class + pure presentation helpers (labels, categories, output previews, completion summaries, source chips, MCP text-error sniff) - gateway/run.py: wiring — construct stream when flag on, route tool/subagent/reasoning events, drain futures + stop in finally - gateway/display_config.py: tool_progress_native (bool, default off) + tool_progress_native_mode (plan|timeline|dense, default plan) Landmines discovered live (docs claim these are optional — they are not): recipient_team_id and recipient_user_id are required for bot-token streams; task_update REPLACES the card wholesale (omitted fields vanish). Co-authored-by: Minh Nguyen <menhguin@users.noreply.github.com>
Rollover: Slack closes streamed messages after ~5 min
(message_not_in_streaming_state) and caps cumulative message size
(msg_too_long) — both observed live 2026-07-05. Instead of falling back
to markdown mid-turn, close the current card ('⤵ continued below') and
continue on a fresh streaming message, replaying the turn header and any
still-in-progress tasks. Proactive (age 240s / ~10k chars sent) so users
never see an error, and reactive on either error. MAX_ROLLOVERS=20 guard;
other API errors still disable cards entirely.
Reasoning cards: full burst now accumulates in the card's collapsible
details (tail-kept ~1500 chars); title shows the rolling word-boundary
tail. run.py now flushes full unsent delta batches instead of the last
sentence fragment. Rationale: reasoning is signal, tool previews are
clutter — so tool output previews shrink 300→120 (finish cap 400→150,
arg details 500→300), buying size budget for reasoning.
Also: _word_trim helper fixes mid-word truncation ('run.py first.I've').
Co-authored-by: Minh Nguyen <menhguin@users.noreply.github.com>
… default Four new display settings, resolved like any other Hermes display param (display.platforms.slack.<key> → display.<key> → built-in default): tool_progress_native_rollover_age_s (default 240) tool_progress_native_rollover_chars (default 10000) tool_progress_native_reasoning_chars (default 0 = uncapped) tool_progress_native_output_chars (default 120) Reasoning cap now defaults to UNCAPPED — reasoning is the highest-value card content. An absolute ceiling of 11k (just under Slack's documented 12k markdown_text field limit) still applies so one card can't blow the message; rollover handles the cumulative budget. Size accounting fixed to net-delta per card id: task_update REPLACES its card, so only growth counts. Raw summing would explode on 💭 cards (each update re-sends the whole burst) and trigger spurious rollovers. Co-authored-by: Minh Nguyen <menhguin@users.noreply.github.com>
…unk joins
The flush timer and tool-call finalize land at arbitrary stream
positions, so cards ended/started mid-sentence — reading as truncation
('…cron reminders.' / next card starting 'tasks are due…', the sentence
split across cards). Flushes now send only the complete-sentence prefix
of the unsent buffer; the incomplete tail is held for the next flush,
and finalize sends the remainder to the same card (the burst is over —
the text belongs there, not on the next card).
Whitespace: probe NousResearch#4 measured Slack's join behavior — trailing spaces
at chunk boundaries are preserved, leading spaces can be stripped at
element boundaries (the 'reminders.Both' jam). Deltas now join with a
trailing space instead of a leading one.
Co-authored-by: Minh Nguyen <menhguin@users.noreply.github.com>
The model always completes its reasoning before emitting a tool call — strictly sequential generation — so when tool.started fires, the full thought already exists. But up to 2s of its tail could be sitting in run.py's flush-throttle buffer, and it flushed AFTER the tool card, landing on a fresh 💭 card: the thought visually split across the Exec entry (user-diagnosed 2026-07-06: 'it's not possible for it to start exec before it finished writing its plan — our sequence is messing up'). tool.started now drains the pending buffer first; the FIFO send lock preserves schedule order, so the card reads thought ✓ → tool with the complete thought on one card. Co-authored-by: Minh Nguyen <menhguin@users.noreply.github.com>
…etails since append-semantics fix) Co-authored-by: Minh Nguyen <menhguin@users.noreply.github.com>
Co-authored-by: Minh Nguyen <menhguin@users.noreply.github.com>
…(YOLO experiment) Header: cards now titled 'callsign · sess6 · HH:MM — <categories>' so multiple streams in one thread are attributable. Subagents: each delegate_task child opens its OWN streamed message (own SlackTaskStream, header '🔀 subagent #N · <goal>'), tools rendering as first-class task entries on its card — main card + one live card per child, updating in parallel. Falls back to the single-card-on-main- stream rendering if the child stream can't open. Turn finally closes any child streams left open (crashed children can't freeze cards). Co-authored-by: Minh Nguyen <menhguin@users.noreply.github.com>
…-first headers Per Minh review of live test: (1) subagent.complete relay carries summary/duration/status — surface the child's result summary as the output field on a final 'result' entry (cards previously showed tool trail but no outcome); (2) header order now identity-first: '🔀 SUBAGENT #N · HH:MM · <goal>' (caps callsign for children; main agent keeps its lowercase config callsign). Co-authored-by: Minh Nguyen <menhguin@users.noreply.github.com>
…ough (relay already carried them) Co-authored-by: Minh Nguyen <menhguin@users.noreply.github.com>
…endered as empty output bullets A/B from live round-2 test: summary starting '**bold**' rendered on the card; summary starting '## Summary' rendered an EMPTY bullet — Slack's rich-text output field swallows leading heading markup. Strip heading markers and collapse newlines before sending. Co-authored-by: Minh Nguyen <menhguin@users.noreply.github.com>
… details Output previews get skimmed past (Minh 2026-07-06) — reasoning is the signal. OUTPUT_PREVIEW_CHARS <= 0 now disables them; config default 0, re-enable via tool_progress_native_output_chars. Subagent result summaries move from output to details so they survive the disable. Co-authored-by: Minh Nguyen <menhguin@users.noreply.github.com>
…ubagent cards Thread position = chat.startStream COMPLETION order, and main + child streams raced their first HTTP calls (observed live: SUBAGENT NousResearch#2 above main, NousResearch#1 below). Child creation now schedules an ordered opener behind a FIFO asyncio.Lock: main's stream is ensured open first, then children open strictly in relay order (= task order). Deterministic layout: main, NousResearch#1, NousResearch#2, ... Co-authored-by: Minh Nguyen <menhguin@users.noreply.github.com>
…s matches thread position Round 2 of the ordering race: the FIFO open gate put main first, but child-vs-child order still followed relay arrival (per-child worker thread scheduling — NousResearch#2's thread can fire subagent.start before NousResearch#1's, observed live). Numbering by task_index therefore couldn't match thread position. Now the card number is drawn from a counter INSIDE the open lock: open order == number order == thread order, by construction. Child task/finish coroutines gate on a per-child open Event so nothing lazily opens a stream around the ordering lock (self-open fallback after 15s so a failed open can't wedge rendering). Verified: 50-trial async simulation of racing opens+events — main first, numbers == thread positions in all trials. Co-authored-by: Minh Nguyen <menhguin@users.noreply.github.com>
…s number at open time) Co-authored-by: Minh Nguyen <menhguin@users.noreply.github.com>
…ation markers, tail-only reasoning replay Three related fixes for subagent cards duplicating after stream rollover (user-reported 2026-07-20, thread 1784483152.731389): 1. Turn-end cleanup only stops streams whose child emitted subagent.complete. Background children outliving the turn keep their streams; stopping them mid-flight forced footer-less reactive rollovers → orphan continuation cards below the final reply (log: 3x 'closing old stream failed' at 01:38:08-18). 2. Continuation cards self-identify: header gets a ⤵ prefix after any rollover (both in _refresh_turn_header and the rollover header replay). header_label (SUBAGENT #N · HH:MM) is preserved, so numbers never change across continuations — out-of-order continuation messages stay attributable instead of reading as renumbering. 3. Rollover replays only the TAIL (500 chars) of an open 💭 card — the full burst already lives on the closed card above; wholesale replay was the 'thinking blocks repeat' symptom. Local full copy kept. tests/gateway/test_slack.py: 216 passed. Takes effect on gateway restart. Co-authored-by: Minh Nguyen <menhguin@users.noreply.github.com>
0b4cb5d to
5446531
Compare
…he task-cards branch The task-cards branch carried an older revision of the shared agent-core reasoning fixes: extract_reasoning() used list-membership (`summary not in reasoning_parts`) instead of substring containment, so a streamed response whose reasoning arrives BOTH as the accumulated string AND chunked across reasoning_details blocks stored the reasoning doubled. NousResearch#59009 fixed this during review; the two branches then diverged. Two tests in TestReasoningDeltasFiredFlag failed on this branch while passing on NousResearch#59009 — that was the tell. Ports agent_runtime_helpers.py, chat_completion_helpers.py, run_agent.py and the dedup regression test from NousResearch#59009 so both PRs share one implementation. 103 tests pass (test_reasoning_command + dedup_59009 + onepassword). Co-authored-by: Minh Nguyen <menhguin@users.noreply.github.com>
|
Rebased onto Conflicts: one, trivial. Also fixed a divergence between this PR and #59009. Two tests in Obsolescence check re-run against the new base (the reason this feature might not be wanted): Verification: 103 tests pass ( Still opt-in behind |
Unblocks the check-attribution CI job, which hard-fails on any author email lacking a contributors/emails/ mapping. Every commit on this branch is authored by carnie[bot] <carnie-bot@openclaw.local> — Minh Nguyen's (@menhguin) agent identity, which authors commits on his behalf. Follows the existing precedent for agent/local-domain emails already mapped in that directory (agent@hermes.dev, agent@agents-Mac-mini.local, 87degrees@87ui-Macmini.local). Added via scripts/add_contributor.py as the file header instructs (not by editing the frozen AUTHOR_MAP in scripts/release.py). One file per email, so it cannot merge-conflict. Note the other failing check on this PR — 'Python lints / Windows footguns (blocking)' — is unrelated to this branch: it flags scripts/tool_search_livetest2.py:190, an upstream file absent from this branch, which teknium1 already fixed in 0e28087 on 2026-07-26 after this PR's CI last ran. Verified locally by merging this branch into current upstream main and running the real checker: '✓ No Windows footguns found (818 file(s) scanned)', exit 0. This push re-triggers CI, which should clear it. Co-authored-by: Minh Nguyen <menhguin@users.noreply.github.com>
Thanks for the contribution! |
Builds on the salvaged typing_status_text plumbing (PR NousResearch#62007): instead of a static 'is thinking...', Slack's assistant status line now updates live as the agent works — 'is running pytest tests/…', 'is reading docs/api.md…' — and reverts to the static text between tool calls. Mechanics: - agent/display.py: build_status_phrase() derives a <=49-char present- tense phrase from the existing _TOOL_VERBS table (+ 'is using <name>' for plugin/MCP tools; None for _thinking). - base adapter: supports_status_text capability flag + set_status_text() per-chat store, cleared when the typing loop winds down. - Slack adapter: send_typing() renders the live phrase when set, falling back to typing_status_text then 'is thinking...'. - gateway/run.py: progress_callback stashes the phrase on tool.started and clears on tool.completed. Rendering rides the existing _keep_typing refresh cadence — zero additional Slack API calls, no rate-limit exposure. Works with tool_progress: off (Slack default); the callback is now armed whenever the adapter supports status text. - display.live_status config (full|verb|off, default full): 'verb' hides argument previews for shared/customer-facing channels. Also fixes a latent crash in the cherry-picked from_dict: malformed non-dict 'extra' sections broke typing_status_text resolution (uses the already-coerced extra dict). Design notes: status text is a side-effect display channel only — never enters the transcript, no prompt-cache impact. Lifecycle guarantees from the stuck-status fix family are preserved (per-thread tracking, clear-on-finish via existing stop_typing paths). Related: NousResearch#45109 (closed; same direction via lifecycle states), NousResearch#59010/NousResearch#51363 (native task cards — complementary, larger scope).
|
Heads-up: the base capability here — Slack-native task cards for tool progress via chat.startStream (opt-in, ID-correlated, with text fallback) — landed in PR #85476, built on the earlier #29496. This PR's larger scope (interleaved reasoning cards, per-subagent streams, identity headers, source chips) is NOT covered by that merge; if you want to pursue those, a rebase onto current main scoped to the deltas beyond #85476 would be the path. The bundled #59009 dedup fix is best kept in its own PR. |


What
Renders tool-call progress on Slack as native collapsible task cards (
chat.startStream/chat.appendStream/chat.stopStreamwithtask_updatechunks — the UI Slack's own AI features use) instead of markdown progress bubbles.One grouped card per turn (
planmode default): per-tool entries with friendly verbs + descriptive completion titles · source-link chips on web tools · interleaved 💭 cards carrying the model's full reasoning (first-sentence TLDR title; complete text in the collapsible body). Cards carry an identity header (<callsign> · <session> · HH:MM — <activity summary>). Eachdelegate_taskchild opens its own streamed message (🔀 SUBAGENT #N · HH:MM · <goal>) — child tools render with arg previews at full parity with the main card, completion carries the child's result summary, and parallel children update independently (verified live with concurrent batches); falls back to a single card on the main stream if a child stream can't open.Opt-in and strictly additive:
display.platforms.slack.tool_progress_nativedefaults tofalse— flag off is byte-identical to current behavior on every platform. Any API failure self-disables and falls back silently to the existing markdown path mid-turn.Why
Slack is the only major platform where Hermes tool progress renders as plain text edits while the platform ships a first-class streaming/task-card API for exactly this. With reasoning on-card, gateway observability roughly matches the Claude/ChatGPT web apps.
Measured Slack API behavior this encodes (docs are wrong or silent on each)
recipient_team_idandrecipient_user_idare required for bot-token streams (docs say optional) —missing_recipient_*otherwise.task_updatesemantics are mixed:title/statusREPLACE per update;detailsAPPENDS across updates with the same id (probe: detailsAAAthenBBB→ storedAAABBB). Alldetailssends here are deltas.message_not_in_streaming_state. Handled by proactive rollover: close with a "⤵ continued below" footer, continue on a fresh streamed message, replay open tasks (default roll at 240s).msg_too_longis per-chunk (single oversized field), not cumulative — 62k cumulative content verified clean;detailsaccepts ≥32k per chunk; titles cap ~256.output/detailsvalue beginning with a markdown heading (## …) renders as an empty element (A/B verified: bold-first text renders fine) — leading heading markers must be stripped.Probe scripts available on request (happy to add as test fixtures).
Delivery correctness
asyncio.Lock(parallel tool events otherwise racestartStreamor reorder status updates).tool.starteddrains the reasoning throttle buffer before the tool card is scheduled — the model completes its reasoning before emitting a tool call, so the buffered tail belongs before the tool entry.complete+ "✗ failed", not Slack'serrorstatus — the red triangle reads as agent breakage when a failed call is routine; it stays reserved for genuine stream abandonment. MCP text-form errors are sniffed so a 403 doesn't render a ✓.Config
Deliberately minimal — three keys, resolving via the standard display-setting chain. Operational tunables (rollover thresholds, reasoning cap) are documented class constants on
SlackTaskStreamrather than config: they encode measured Slack API physics (the ~306s stream lifetime, per-chunk size caps), not user preferences, so exposing them would only add support surface. Rollover fires at 290s (~95% of the measured lifetime); a proactive miss falls through to the reactive-rollover path, so the margin is safety-redundant rather than load-bearing.On
tool_progress_native_mode: all three Slack modes work, butplan(the default) renders best — one collapsible group with details expanded inline, and interim prose sections cleanly below the card. In side-by-side testing with identical task streams,timelineanddensecurrently render near-identically once tasks complete (one bordered block per task, details collapsed) — the knob is passed through for forward-compat with Slack's renderer rather than because the modes are visually distinct today.Files
gateway/slack_task_stream.py— new, self-contained (no gateway imports):SlackTaskStreamlifecycle + pure presentation helpers.gateway/run.py— wiring only; every added path is flag-gated and exception-wrapped.gateway/display_config.py— toggle + 5 tuning knobs.How to test
Enable the flag on one channel, then: short turn → single clean card; long turn (>5 min) → seamless rollover;
delegate_taskbatch → numbered live subagent cards; reasoning-heavy turn → 💭 cards with full text in the body. Unit suite (fake Slack client, ~30 scenarios: rollover reactive/proactive + replay, append-semantics deltas, fragment gating, sentence-boundary flushing, config knobs, self-disable, runaway guard) ready to be added undertests/gateway/— will port to pytest in this PR if the approach looks right to you.Platforms tested
macOS arm64, Hermes 2026.7.1, live production Slack workspace — all five API behaviors above verified against production Slack over ~2 days of heavy real-agent usage.
Related PRs
#17321 / #29496 — earlier takes on this surface (streamed text lines / minimal cards). #54522 — same feature via
GatewayEventDispatcher; this PR hooks the shippedtool_progress_callbackfan instead because the dispatcher isn't wired in a release yet and has no reasoning/subagent event types — happy to migrate onto that seam as a follow-up once it lands. #48066 — final-answer draft streaming; complementary (different transport), stacks cleanly on top of this.Depends on the reasoning-delivery fixes in #59009 (the 💭 cards surface reasoning to users, which is how both agent-core bugs were found).