Slack: dedupe partial streaming replies by gumadeiras · Pull Request #62859 · openclaw/openclaw

gumadeiras · 2026-04-08T02:43:54Z

Summary

add turn-local dedupe for Slack partial-stream delivery on the same thread
suppress fallback re-sends when the existing preview already matches the final text
cover the new partial-stream guards with focused Slack regression tests

Testing

pnpm test extensions/slack/src/draft-stream.test.ts extensions/slack/src/monitor/message-handler/dispatch.streaming.test.ts

greptile-apps · 2026-04-08T02:46:26Z

Greptile Summary

This PR adds turn-local deduplication for Slack partial-stream delivery: a createSlackTurnDeliveryTracker (keyed on {threadTs, text, mediaUrls, blocks}) guards all three delivery paths — normal, stream-start, and stream-append — and a new lastText() accessor on SlackDraftStream powers a preview-match suppression when finalizeSlackPreviewEdit fails but the preview already shows the final text.

Confidence Score: 5/5

Safe to merge; deduplication logic is correct and well-tested, with no P0/P1 findings.

All findings are P2 (minor style). The tracker key construction is deterministic, state-variable consistency is preserved across suppression paths, and the lastText() fallback correctly handles the empty-string/undefined boundary. Tests cover the new tracker and the lastText accessor.

No files require special attention.

Vulnerabilities

No security concerns identified.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: extensions/slack/src/monitor/message-handler/dispatch.ts
Line: 541-543

Comment:
**Redundant `!reply.hasMedia` guard in catch block**

`canFinalizeViaPreviewEdit` already asserts `!reply.hasMedia` before entering this branch, so the inner check on line 541 can never be `true` when `reply.hasMedia` is truthy. The condition is harmless but adds noise.

```suggestion
          if (
            !slackBlocks?.length &&
            normalizedPreviewText &&
            normalizedPreviewText === normalizeSlackOutboundText(trimmedFinalText)
          ) {
```

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "Slack: dedupe partial streaming replies" | Re-trigger Greptile}

Copilot

Pull request overview

This PR improves Slack partial streaming reliability by deduplicating repeated deliveries within a single dispatch turn and by avoiding unnecessary fallback re-sends when the preview already matches the final reply text.

Changes:

Add a turn-local delivery tracker to suppress duplicate normal and streaming (start/append) sends on the same Slack thread.
Skip fallback sending when the existing preview text already matches the final normalized outbound text.
Add regression tests for the new delivery tracker and for draft stream preview text visibility.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
extensions/slack/src/monitor/message-handler/dispatch.ts	Adds turn-local delivery dedupe and preview/fallback suppression logic for streaming + preview finalize paths.
extensions/slack/src/monitor/message-handler/dispatch.streaming.test.ts	Adds a focused unit test for the new turn delivery tracker behavior.
extensions/slack/src/draft-stream.ts	Exposes `lastText()` on the draft stream API for downstream finalize/fallback logic.
extensions/slack/src/draft-stream.test.ts	Adds a test asserting `lastText()` reflects the latest preview content after a flush.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 73a7d52746

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

aisle-research-bot · 2026-04-08T20:45:21Z

🔒 Aisle Security Analysis

We found 1 potential security issue(s) in this PR:

#	Severity	Title
1	🟡 Medium	Potential per-turn memory/CPU DoS from unbounded Slack delivery deduplication keys

1. 🟡 Potential per-turn memory/CPU DoS from unbounded Slack delivery deduplication keys

Property	Value
Severity	Medium
CWE	CWE-400
Location	`extensions/slack/src/monitor/message-handler/dispatch.ts:126-160`

Description

The new per-turn delivery de-duplication uses a Set<string> of keys computed by JSON.stringify(...) over potentially large reply content.

buildSlackTurnDeliveryKey stringifies text, mediaUrls, and full Slack blocks objects to build a key.
The tracker stores every distinct key for the lifetime of a single dispatchPreparedSlackMessage call.
In streaming mode, textOverride is the streamed text chunk; if many distinct chunks are produced in one turn, the Set will grow linearly with the number of chunks.
If Slack blocks are present, they are included verbatim in the JSON key; while block count is capped elsewhere, block objects can still be large, and repeatedly stringifying them adds CPU overhead.

This can enable a denial-of-service scenario (high CPU due to repeated stringify and increased memory usage from the growing Set) if an attacker can induce many unique payloads/updates in a single turn (e.g., very long/rapid streaming or repeated slightly-different tool outputs).

Recommendation

Avoid building/storing de-duplication keys from full payload bodies.

Safer options:

Bound the tracker (caps worst-case memory): keep only the last N keys per turn/thread.
Use a fixed-size digest instead of JSON.stringify output (caps key size and reduces memory pressure). For example, hash a normalized representation with a size limit:

import { createHash } from "node:crypto";

function safeKey(input: unknown): string {
  const json = JSON.stringify(input);
  const capped = json.length > 10_000 ? json.slice(0, 10_000) : json;
  return createHash("sha256").update(capped).digest("hex");
}

For streaming dedupe, consider only tracking the last delivered streamed text per thread (or a rolling checksum) instead of accumulating all chunks.

Additionally, consider rejecting/limiting unusually large blocks payloads before they reach the keying logic.

Analyzed PR: #62859 at commit cbecb50

_{Last updated on: 2026-04-08T21:56:59Z}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b33084b2d3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

gumadeiras · 2026-04-08T21:18:55Z

Triage update on automated feedback:

Fixed the valid delivery-routing issue: turn-local Slack dedupe now distinguishes explicit replyToId targets, so identical payloads aimed at different explicit reply targets do not collide.
Resolved the related review threads.

Security-bot notes:

The reported per-turn Set growth is not being treated as a security issue for this PR. Per SECURITY.md, we require a trust-boundary bypass plus demonstrated impact. This tracker is in-memory and scoped to a single dispatch turn; the comment did not demonstrate a boundary bypass.
The JSON.stringify claim is likewise not accepted as a security issue here. At most that would be trusted-plugin robustness/hardening around plugin-owned Slack block payloads, not a demonstrated security boundary break under this repo's trust model.

No further action on those security-bot comments in this PR.

gumadeiras · 2026-04-08T21:55:33Z

Merged via squash.

Prepared head SHA: cbecb50c06f7657dc259cf9fdc71419b9b390559
Merge commit: 10c87527d5d85c42b3733f5b52e95c414b1e0c7a

Thanks @gumadeiras!

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cbecb50c06

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@gumadeiras

Merged via squash. Prepared head SHA: cbecb50 Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Reviewed-by: @gumadeiras

@gumadeiras

Merged via squash. Prepared head SHA: cbecb50 Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Reviewed-by: @gumadeiras

Copilot AI review requested due to automatic review settings April 8, 2026 02:43

openclaw-barnacle Bot added channel: slack Channel integration: slack size: S maintainer Maintainer-authored PR labels Apr 8, 2026

Copilot started reviewing on behalf of gumadeiras April 8, 2026 02:44 View session

greptile-apps Bot reviewed Apr 8, 2026

View reviewed changes

Comment thread extensions/slack/src/monitor/message-handler/dispatch.ts Outdated

Copilot AI reviewed Apr 8, 2026

View reviewed changes

Comment thread extensions/slack/src/draft-stream.ts

Comment thread extensions/slack/src/monitor/message-handler/dispatch.ts Outdated

chatgpt-codex-connector Bot reviewed Apr 8, 2026

View reviewed changes

Comment thread extensions/slack/src/monitor/message-handler/dispatch.ts

Comment thread extensions/slack/src/monitor/message-handler/dispatch.ts Outdated

gumadeiras self-assigned this Apr 8, 2026

openclaw-barnacle Bot added size: M and removed size: S labels Apr 8, 2026

chatgpt-codex-connector Bot reviewed Apr 8, 2026

View reviewed changes

Comment thread extensions/slack/src/monitor/message-handler/dispatch.ts

gumadeiras added 4 commits April 8, 2026 17:53

Slack: dedupe partial streaming replies

94e24fa

Slack: preserve fallback sends after preview edit failures

7cad50d

Slack: simplify preview fallback test setup

2385be9

Slack: key delivery dedupe by explicit reply target

cbecb50

gumadeiras force-pushed the codex/slack-partial-stream-dedupe branch from 5b2fb2e to cbecb50 Compare April 8, 2026 21:55

gumadeiras merged commit 10c8752 into main Apr 8, 2026
9 checks passed

gumadeiras deleted the codex/slack-partial-stream-dedupe branch April 8, 2026 21:55

chatgpt-codex-connector Bot reviewed Apr 8, 2026

View reviewed changes

Comment thread extensions/slack/src/monitor/message-handler/dispatch.ts

github-actions Bot mentioned this pull request Apr 8, 2026

📡 Upstream Digest — 2026-04-08 22:31 UTC curtismercier/openclaw-mods#514

Open

github-actions Bot mentioned this pull request Apr 13, 2026

Update ghcr.io/openclaw/openclaw Docker tag to v2026.5.7 claytono/infra#1977

Open

1 task

Uh oh!

Conversation

gumadeiras commented Apr 8, 2026

Summary

Testing

Uh oh!

greptile-apps Bot commented Apr 8, 2026

Greptile Summary

Confidence Score: 5/5

Vulnerabilities

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

aisle-research-bot Bot commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔒 Aisle Security Analysis

Description

Recommendation

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

gumadeiras commented Apr 8, 2026

Uh oh!

Uh oh!

gumadeiras commented Apr 8, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

aisle-research-bot Bot commented Apr 8, 2026 •

edited

Loading