fix(slack): keep one draft message in progress mode#85612
Conversation
|
Codex review: needs maintainer review before merge. Reviewed May 31, 2026, 12:00 PM ET / 16:00 UTC. Summary PR surface: Source +3, Tests +42. Total +45 across 2 files. Reproducibility: yes. source-reproducible with supplied live proof: current main calls forceNewMessage at assistant/reasoning boundaries, and that clears the Slack draft message id so the next progress update sends a new message. I did not run a new live Slack reproduction from this checkout. Review metrics: none identified. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Mantis proof suggestion Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the focused progress-mode guard after required checks are green or classified and any overlapping Slack dispatch PRs are reconciled, while keeping partial/replace draft-boundary behavior unchanged. Do we have a high-confidence way to reproduce the issue? Yes, source-reproducible with supplied live proof: current main calls forceNewMessage at assistant/reasoning boundaries, and that clears the Slack draft message id so the next progress update sends a new message. I did not run a new live Slack reproduction from this checkout. Is this the best way to solve the issue? Yes. The PR is the narrowest maintainable fix I found: it skips only the progress-mode message-id reset, keeps boundary cleanup, and adds regression coverage that partial/replace mode still starts a new draft. AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against 29dd7847fd96. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +3, Tests +42. Total +45 across 2 files. View PR surface stats
Acceptance criteria:
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
|
ClawSweeper PR egg ✨ Hatched: 🥚 common Frosted Shellbean Hatch commandComment Hatchability rules:
Rarity: 🥚 common. What is this egg doing here?
|
|
@clawsweeper re-review |
|
🦞👀 Command router queued. I will update this comment with the next step. |
|
🦞🧹 Reason: re-review requires an open issue or PR. |
32c8b5b to
fd7d44f
Compare
05c24ca to
41e304a
Compare
This comment was marked as spam.
This comment was marked as spam.
41e304a to
947a719
Compare
|
Maintainer proof on refreshed head 947a719: Behavior addressed: Slack progress-mode drafts now stay on one rolling preview message across assistant/reasoning boundaries, while partial/replace modes still force a new draft at assistant boundaries. Real environment tested: contributor's production Slack proof remains in the PR body; maintainer local proof covered the Slack dispatch boundary behavior on the refreshed head. Exact steps or command run after this patch:
Evidence after fix: focused Vitest passed 87 tests in the Slack dispatch preview/streaming shard; format, lint, and diff check passed; autoreview returned clean after one accepted finding was fixed. PR body includes live before/after Slack screenshots from the contributor's production workspace. Observed result after fix: status_final skips only the draft message-id reset, keeps boundary cleanup, preserves last non-empty progress lines for rich and text renderers, and keeps partial/replace forceNewMessage behavior covered by regression test. What was not tested: I did not run a new live Slack send from this maintainer checkout. |
Keep Slack progress-mode drafts on one rolling preview message across assistant and reasoning boundaries while preserving boundary cleanup and the latest visible tool-progress lines. Partial/replace modes still start a fresh draft at assistant boundaries. Co-authored-by: Mike Harrison <[email protected]>
947a719 to
8286d36
Compare
|
Refreshed maintainer proof on exact head 8286d36: Behavior addressed: Slack progress-mode drafts stay on one rolling preview message across assistant/reasoning boundaries, while partial/replace modes still force a new draft at assistant boundaries. Real environment tested: contributor's production Slack proof remains in the PR body; maintainer local proof covered the Slack dispatch boundary behavior on the rebased head. Exact steps or command run after this patch:
Evidence after fix: focused Vitest passed 87 tests; format, lint, diff check, and the moving-base gateway lint probe passed; autoreview returned clean after the accepted text-renderer progress finding was fixed. Observed result after fix: status_final skips only the draft message-id reset, keeps boundary cleanup, preserves last non-empty progress lines for rich and text renderers, and keeps partial/replace forceNewMessage behavior covered by regression test. What was not tested: I did not run a new live Slack send from this maintainer checkout. |
Keep Slack progress-mode drafts on one rolling preview message across assistant and reasoning boundaries while preserving boundary cleanup and the latest visible tool-progress lines. Partial/replace modes still start a fresh draft at assistant boundaries. Co-authored-by: Peter Steinberger <[email protected]>
Keep Slack progress-mode drafts on one rolling preview message across assistant and reasoning boundaries while preserving boundary cleanup and the latest visible tool-progress lines. Partial/replace modes still start a fresh draft at assistant boundaries. Co-authored-by: Peter Steinberger <[email protected]>
Keep Slack progress-mode drafts on one rolling preview message across assistant and reasoning boundaries while preserving boundary cleanup and the latest visible tool-progress lines. Partial/replace modes still start a fresh draft at assistant boundaries. Co-authored-by: Peter Steinberger <[email protected]>
Summary
In
channels.slack.streaming.mode = "progress"(internallystatus_final), Slack should keep one rolling draft preview and finalize that same message in place when the final reply lands. Multi-segment assistant turns were instead posting one draft per assistant/reasoning boundary because the boundary handler calleddraftStream.forceNewMessage(), clearing the draftmessageIdbefore the next update.Fix
status_finaldrafts on the same Slack message by skipping only theforceNewMessage()call in progress mode.Verification
node scripts/run-vitest.mjs extensions/slack/src/monitor/message-handler/dispatch.preview-fallback.test.ts extensions/slack/src/monitor/message-handler/dispatch.streaming.test.ts --reporter=dot./node_modules/.bin/oxfmt --check extensions/slack/src/monitor/message-handler/dispatch.ts extensions/slack/src/monitor/message-handler/dispatch.preview-fallback.test.ts./node_modules/.bin/oxlint --tsconfig ./tsconfig.json extensions/slack/src/monitor/message-handler/dispatch.ts extensions/slack/src/monitor/message-handler/dispatch.preview-fallback.test.tsgit diff --check/Users/steipete/Projects/agent-scripts/skills/autoreview/scripts/autoreview --mode localReal behavior proof
Behavior or issue addressed: In
channels.slack.streaming.mode = "progress", a multi-segment assistant turn was posting one new draft message per assistant segment instead of editing one rolling preview in place. Earlier drafts were left as orphaned progress messages above the final reply.Real environment tested: Production Slack workspace, real Slack bot identity via socket mode, OpenClaw agent running the patched bundle on the same EC2 host that reproduced the bug. Slack streaming config used
channels.slack.streaming = { mode: "progress" }per account, with replies threaded.Exact steps or command run after this patch: Built the patched Slack extension bundle, deployed it onto the production agent host, restarted the agent so Slack reconnected with the new dispatch wiring, then sent the same kind of tool-using Slack prompt that previously produced multiple progress messages. The channel was observed during streaming and after the final reply landed.
Evidence after fix: Before screenshot showed one turn producing multiple orphaned progress messages. After screenshot showed one rolling progress draft finalized in place, with no orphaned progress messages above the final reply. Evidence images: https://github.com/mycarrysun/openclaw/releases/download/slack-progress-proof/slack-progress-before.jpg and https://github.com/mycarrysun/openclaw/releases/download/slack-progress-proof/slack-progress-after.jpg.
Observed result after fix: The Slack channel showed exactly one draft preview message for the whole multi-segment turn. Assistant text and tool-progress updates edited that single message instead of posting new drafts, and the final reply finalized the same message.
What was not tested: Partial and block/replace streaming modes were not exercised against a live Slack workspace in this run; focused regression tests cover that those modes still force a new draft on assistant boundaries.