PR #65077: fix(slack): keep reply suppression thread-aware

• Repository: openclaw/openclaw
• Author: neeravmakwana
• State: closed | merged: False
• Link: https://github.com/openclaw/openclaw/pull/65077

Description (problem / solution / changelog)

Summary

• Problem: generic message tool send tracking dropped threadId, so same-channel Slack sends were treated as the same destination even when they targeted a different thread.
• Why it matters: a threaded send could suppress the final reply for another Slack thread in the same channel, which is the wrong conversation boundary.
• What changed: preserved threadId for generic message sends and threaded the originating thread id through both direct and followup reply-suppression checks, with focused Slack regression coverage.
• What did NOT change (scope boundary): this does not alter Slack outbound transport, thread routing, or the existing NO_REPLY completion-delivery behavior.

Change Type (select all)

• Bug fix
• Feature
• Refactor required for the fix
• Docs
• Security hardening
• Chore/infra

Scope (select all touched areas)

• Gateway / orchestration
• Skills / tool execution
• Auth / tokens
• Memory / storage
• Integrations
• API / contracts
• UI / DX
• CI/CD / infra

Linked Issue/PR

• Closes #
• Related #65043
• This PR fixes a bug or regression

Root Cause (if applicable)

• Root cause: generic message-tool send extraction recorded provider/account/target but dropped threadId, and the fallback suppression path treated same-channel Slack sends as matching even when the originating turn was in a different thread.
• Missing detection / guardrail: we had same-target suppression coverage, but no Slack regression test proving that same channel plus different thread must not suppress.
• Contributing context (if known): the current #65043 duplicate top-level-post path appears fixed on main; this patch hardens the remaining thread-aware suppression edge in the same Slack/threaded completion family.

Regression Test Plan (if applicable)

• Coverage level that should have caught this:
  • Unit test
  • Seam / integration test
  • End-to-end test
  • Existing coverage already sufficient
• Target test or file: src/agents/pi-embedded-subscribe.tools.extract.test.ts, src/auto-reply/reply/reply-payloads.test.ts, src/auto-reply/reply/followup-delivery.test.ts, src/auto-reply/reply/agent-runner.misc.runreplyagent.test.ts
• Scenario the test should lock in: Slack same-channel sends only suppress replies when the thread id also matches; different-thread sends in the same channel must still allow the final reply.
• Why this is the smallest reliable guardrail: the bug spans tool-send extraction plus both direct and queued reply-suppression paths, so these focused tests lock each seam without needing live Slack coverage.
• Existing test that already covers this (if any): existing same-target suppression tests covered provider/account matching but not Slack thread mismatches.
• If no new test is added, why not: N/A

User-visible / Behavior Changes

• Slack threaded agent runs no longer let a message tool send in one thread suppress the final reply for a different thread in the same channel.

Diagram (if applicable)

Before:
[Slack thread A send via message tool] -> [same channel match] -> [thread B final reply suppressed]

After:
[Slack thread A send via message tool] -> [channel match + thread mismatch] -> [thread B final reply still delivered]

Security Impact (required)

• New permissions/capabilities? (No)
• Secrets/tokens handling changed? (No)
• New/changed network calls? (No)
• Command/tool execution surface changed? (No)
• Data access scope changed? (No)
• If any Yes, explain risk + mitigation:

Repro + Verification

Environment

• OS: macOS 15 / Darwin 25.3.0
• Runtime/container: local repo checkout
• Model/provider: N/A
• Integration/channel (if any): Slack reply suppression path
• Relevant config (redacted): Slack channel replies with threaded agent runs / message tool sends

Steps

1. Start in one Slack thread and trigger a message tool send to that thread.
2. Continue a separate Slack thread in the same channel.
3. Let the runner evaluate same-target suppression for the second thread.

Expected

• Only the matching Slack thread can suppress its own reply.
• A different Slack thread in the same channel still receives its final reply.

Actual

• Before this fix, same-channel comparison could suppress across Slack threads because the generic message tool send metadata did not keep threadId.

Evidence

• Failing test/log before + passing after
• Trace/log snippets
• Screenshot/recording
• Perf numbers (if relevant)

Human Verification (required)

• Verified scenarios: preserved threadId from generic message sends; Slack same-channel same-thread suppression; Slack same-channel different-thread non-suppression; direct runner suppression path; followup suppression path.
• Edge cases checked: account mismatch still does not suppress; non-threaded same-target suppression remains unchanged; NO_REPLY completion suppression remains covered separately.
• What you did not verify: live Slack manual repro.

Review Conversations

• I replied to or resolved every bot review conversation I addressed in this PR.
• I left unresolved only the conversations that still need reviewer or maintainer judgment.

Compatibility / Migration

• Backward compatible? (Yes)
• Config/env changes? (No)
• Migration needed? (No)
• If yes, exact upgrade steps:

Risks and Mitigations

• Risk: provider-specific suppression rules could diverge from the generic fallback for threaded channels.
  • Mitigation: the change only tightens the non-plugin fallback path and adds focused Slack regression coverage around both direct and queued delivery.

Additional Notes

• AI-assisted: yes.
• Testing depth: focused regression tests passed, pnpm build passed, pnpm format passed.
• Broader repo gates: pnpm check and the support-boundary lane inside OPENCLAW_VITEST_MAX_WORKERS=1 pnpm test still hit an unrelated existing Codex TypeScript failure in extensions/codex/src/app-server/dynamic-tools.ts (prepareArguments missing on AnyAgentTool).

Made with Cursor

Changed files

• CHANGELOG.md (modified, +2/-0)
• src/agents/pi-embedded-subscribe.tools.extract.test.ts (modified, +25/-0)
• src/agents/pi-embedded-subscribe.tools.ts (modified, +8/-1)
• src/auto-reply/reply/agent-runner-payloads.ts (modified, +2/-0)
• src/auto-reply/reply/agent-runner.misc.runreplyagent.test.ts (modified, +39/-1)
• src/auto-reply/reply/agent-runner.ts (modified, +1/-0)
• src/auto-reply/reply/followup-delivery.test.ts (modified, +53/-0)
• src/auto-reply/reply/followup-delivery.ts (modified, +2/-0)
• src/auto-reply/reply/followup-runner.ts (modified, +1/-0)
• src/auto-reply/reply/reply-payloads-dedupe.ts (modified, +26/-2)
• src/auto-reply/reply/reply-payloads.test.ts (modified, +75/-0)

PR #65577: Outbound: centralize payload normalization semantics

• Repository: openclaw/openclaw
• Author: joshavant
• State: closed | merged: True
• Link: https://github.com/openclaw/openclaw/pull/65577

Description (problem / solution / changelog)

Summary

Describe the problem and fix in 2–5 bullets:

• Problem: outbound payload normalization semantics were duplicated across delivery prep, JSON output shaping, and mirror summary paths.
• Why it matters: duplicated keep/drop and text/media parsing behavior increases recurring drift risk (for silent tokens, MEDIA: directives, reasoning suppression, and metadata preservation), which can cause regressions across channels and call sites.
• What changed: introduced a single channel-agnostic OutboundPayloadPlan authority in src/infra/outbound/payloads.ts and migrated delivery/mirror/JSON call sites to consume plan projections.
• What did NOT change (scope boundary): explicit-send literal behavior, queue raw persistence, adapter/channel transforms, heartbeat/cron semantics, and delivery transport ordering/side effects.

Background / Issue Signal

• Recent issue signal for this hotspot included #65002, #64665, #64976, #65043, #64971, #65315, and #65375 (recurring outbound text/media/silent/routing drift across entry paths).
• Hotspot evidence: src/infra/outbound/deliver.ts remains a large seam (~804 LOC), historically combining normalization concerns with transport concerns.
• Cross-cutting semantics have broad fan-out (resolveSendableOutboundReplyParts appears in ~30 non-test usages, isSilentReplyText in ~21, and stripHeartbeatToken in ~11), which is where drift pressure comes from.
• This PR centralizes outbound normalization at the outbound seam while intentionally keeping heartbeat/cron-specific semantics separate, consistent with the approved constraints.

Change Type (select all)

• Bug fix
• Feature
• Refactor required for the fix
• Docs
• Security hardening
• Chore/infra

Scope (select all touched areas)

• Gateway / orchestration
• Skills / tool execution
• Auth / tokens
• Memory / storage
• Integrations
• API / contracts
• UI / DX
• CI/CD / infra

Linked Issue/PR

• Closes #
• Related #
• This PR fixes a bug or regression

Root Cause (if applicable)

• Root cause: outbound normalization logic evolved in multiple places with overlapping responsibilities, so behavior updates could land in one path but not others.
• Missing detection / guardrail: insufficient cross-path characterization tests tying delivery, mirror, and JSON behavior to the same expected semantics.
• Contributing context (if known): multiple independent call sites (deliver.ts, send.ts, message.ts, agents/command/delivery.ts) each needed normalization outputs.

Regression Test Plan (if applicable)

• Coverage level that should have caught this:
  • Unit test
  • Seam / integration test
  • End-to-end test
  • Existing coverage already sufficient
• Target test or file:
  • src/infra/outbound/payloads.test.ts
  • src/infra/outbound/deliver.test.ts
  • src/infra/outbound/message.test.ts
• Scenario the test should lock in:
  • silent token exact/mixed/JSON keep/drop semantics
  • media + silent combinations
  • directive parsing (MEDIA:, reply tags, audioAsVoice)
  • reasoning suppression
  • channelData-only handling
  • mirror parity and queue raw persistence
• Why this is the smallest reliable guardrail: these tests assert behavior at the normalization boundary and at transport/mirror seams where drift previously occurred.
• Existing test that already covers this (if any): existing outbound delivery/message/send tests were extended and retained.
• If no new test is added, why not: N/A

User-visible / Behavior Changes

None intended. This is behavior-preserving centralization with stronger regression coverage.

Diagram (if applicable)

Before:
caller -> local normalization variant A/B/C -> delivery/json/mirror

After:
caller -> OutboundPayloadPlan (single normalization authority) -> projection (delivery/json/mirror)

Security Impact (required)

• New permissions/capabilities? (Yes/No): No
• Secrets/tokens handling changed? (Yes/No): No
• New/changed network calls? (Yes/No): No
• Command/tool execution surface changed? (Yes/No): No
• Data access scope changed? (Yes/No): No
• If any Yes, explain risk + mitigation: N/A

Repro + Verification

Environment

• OS: macOS
• Runtime/container: Node 24 + pnpm workspace
• Model/provider: N/A
• Integration/channel (if any): outbound channel adapters (test doubles)
• Relevant config (redacted): test configs only

Steps

1. Run outbound normalization and seam tests.
2. Verify queue write-ahead behavior remains raw.
3. Verify mirror matrix behavior for MEDIA: and silent token variants.

Expected

• Delivery/JSON/mirror semantics stay stable while normalization authority is centralized.

Actual

• Matches expected in targeted suites.

Evidence

Attach at least one:

• Failing test/log before + passing after
• Trace/log snippets
• Screenshot/recording
• Perf numbers (if relevant)

Key verification commands run:

• pnpm test src/infra/outbound/payloads.test.ts src/infra/outbound/deliver.test.ts src/infra/outbound/message.test.ts src/gateway/server-methods/send.test.ts src/agents/command/delivery.test.ts
• pnpm test src/infra/outbound/deliver.test.ts src/infra/outbound/message.test.ts

Human Verification (required)

What you personally verified (not just CI), and how:

• Verified scenarios:
  • plan projections preserve existing normalization behavior for delivery/json/mirror
  • queue persistence stays raw at enqueue boundary
  • sendMessage mirror outcomes for directive/silent-token matrix remain as expected
  • delivery transport flow still runs queue/hook/adapter paths in existing order
• Edge cases checked:
  • exact + JSON silent tokens
  • silent token with media
  • MEDIA: directive extraction
  • channelData-only payload behavior
• What you did not verify:
  • full monorepo pnpm test green state (repository currently has unrelated failing shards outside this change surface)

Review Conversations

• I replied to or resolved every bot review conversation I addressed in this PR.
• I left unresolved only the conversations that still need reviewer or maintainer judgment.

If a bot review conversation is addressed by this PR, resolve that conversation yourself. Do not leave bot review conversation cleanup for maintainers.

Compatibility / Migration

• Backward compatible? (Yes/No): Yes
• Config/env changes? (Yes/No): No
• Migration needed? (Yes/No): No
• If yes, exact upgrade steps: N/A

Risks and Mitigations

• Risk: subtle normalization drift in less-common payload combinations.
  • Mitigation: added characterization + matrix tests with fixed expected outputs at queue and mirror seams.
• Risk: future expansion could accidentally move channel-specific logic into plan.
  • Mitigation: explicit plan boundary comments and keeping adapter transforms in deliver.ts.

Changed files

• src/agents/command/delivery.ts (modified, +6/-4)
• src/gateway/server-methods/send.ts (modified, +10/-13)
• src/infra/outbound/deliver.test.ts (modified, +30/-0)
• src/infra/outbound/deliver.ts (modified, +16/-24)
• src/infra/outbound/message.test.ts (modified, +103/-0)
• src/infra/outbound/message.ts (modified, +10/-10)
• src/infra/outbound/payloads.test.ts (modified, +238/-0)
• src/infra/outbound/payloads.ts (modified, +132/-54)