PR #70370: fix(slack): treat Slack Connect finalize errors as benign in stopSlackStream

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

Description (problem / solution / changelog)

Summary

• Problem: When Slack native streaming is enabled and the recipient is a Slack Connect user, chat.stopStream fails with An API error occurred: user_not_found. OpenClaw logs slack-stream: failed to stop stream: ... via runtime.error in dispatch, which makes every Slack Connect reply look like an error even though the text delivered via chat.appendStream is already visible to the user.
• Why it matters: On any workspace that uses Slack Connect / shared channels, native streaming silently looks broken at the dispatch layer. Operators see a stream of red in the gateway log that is caused by the Slack Connect recipient shape, not a dropped reply.
• What changed: stopSlackStream in extensions/slack/src/streaming.ts now catches session.streamer.stop(...) errors. When the Slack error code is one of user_not_found, team_not_found, or missing_recipient_user_id (all Slack Connect / recipient-resolution cases), swallow the error, keep the session marked as stopped, and log through the existing logVerbose path explaining that appended text is still visible. Any other Slack error still propagates so the caller's existing runtime.error(...) log is preserved.
• What did NOT change (scope boundary): No behavior change for DMs in the host workspace, thread replies to non-Connect users, or non-streaming delivery paths. The dispatch call site in extensions/slack/src/monitor/message-handler/dispatch.ts is unchanged; this PR stays inside the streaming.ts boundary. No other plugin, core, or SDK changes.

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 #70295
• This PR fixes a bug or regression

Root Cause (if applicable)

• Root cause: stopSlackStream in extensions/slack/src/streaming.ts awaited session.streamer.stop(...) without any error handling. The Slack Connect recipient_user_id / team_id path is not resolvable at finalize time, so Slack returns user_not_found (or sibling codes) on chat.stopStream. The error bubbled up through the outer try/catch in dispatch.ts and was logged via runtime.error as if the reply itself had failed.
• Missing detection / guardrail: no classification of finalize errors that are "visible text already delivered" vs "stream actually failed". The caller cannot distinguish the two.
• Contributing context (if known): the issue triage (@rafiki270 on #70295) identified extensions/slack/src/streaming.ts stopSlackStream() as the missing-error-handling site and the finalize failure mode as Slack-Connect-specific.

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: extensions/slack/src/streaming.test.ts (new)
• Scenario the test should lock in: session.streamer.stop() throwing each of the three benign Slack Connect error shapes -> stopSlackStream resolves without throwing and marks the session stopped. An unexpected Slack error code (not_authed) AND a non-Slack-shaped error (new Error("socket reset")) still re-throw so the caller's log path stays intact. Duplicate-stop on an already-stopped session remains a no-op.
• Why this is the smallest reliable guardrail: stopSlackStream is the seam; the dispatch call site is unchanged.
• Existing test that already covers this (if any): none - streaming.ts had no direct unit coverage.
• If no new test is added, why not: N/A (added).

User-visible / Behavior Changes

• Slack Connect replies with native streaming enabled no longer surface a runtime.error("slack-stream: failed to stop stream: user_not_found") line when the stream text was actually delivered.
• Log output shifts these finalize failures to logVerbose with a clear note that appended text remains visible and the stream is treated as stopped.

Diagram (if applicable)

N/A

Security Impact (required)

• New permissions/capabilities? No
• Secrets/tokens handling changed? No
• New/changed network calls? No (we do not issue follow-on calls on finalize failure; the append() call's text is already at Slack).
• Command/tool execution surface changed? No
• Data access scope changed? No
• If any Yes, explain risk + mitigation: N/A

Repro + Verification

Environment

• OS: macOS 15
• Runtime/container: Node 22, pnpm
• Model/provider: any (reply is generated upstream; this is the delivery path)
• Integration/channel (if any): Slack, native streaming enabled
• Relevant config (redacted):

  channels.slack.streaming.mode = "partial"
  channels.slack.streaming.nativeTransport = true
  channels.slack.replyToMode = "first"

Steps

1. Configure OpenClaw Slack with native streaming as above.
2. Trigger a reply in a shared channel/thread that includes a Slack Connect user.

Expected

Reply appears in thread (as it already does via append()) and no slack-stream: failed to stop stream error line appears at the dispatch layer for the user_not_found / team_not_found / missing_recipient_user_id cases.

Actual (before this PR)

[slack] slack-stream: failed to stop stream: Error: An API error occurred: user_not_found at runtime.error level, even when the reply text is visible.

Evidence

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

Local checks before push:

• pnpm test extensions/slack/src/streaming.test.ts: 1 file, 6 tests pass.
• pnpm test extensions/slack: 90 files, 736 tests pass.
• pnpm check:changed --staged (pre-commit): extension lane ran 89 files, 734 tests, all pass.
• pnpm format:check, pnpm lint on touched files: clean.

Human Verification (required)

• Verified scenarios (via unit tests mocking ChatStreamer.stop):
  • user_not_found throw -> stopSlackStream resolves, session marked stopped.
  • team_not_found throw -> same.
  • missing_recipient_user_id throw -> same.
  • Unexpected Slack code (not_authed) -> re-thrown with the original message; session still marked stopped to prevent retry loops.
  • Non-Slack error (socket reset) -> re-thrown unchanged.
  • Duplicate stop on an already-stopped session -> no-op, streamer.stop not called.
  • Full Slack extension lane still green (736 tests).
• Edge cases checked: preserving session.stopped = true on both throw paths so a retry would not re-enter streamer.stop. The error classification reads from err.data.error (shape used by @slack/web-api errors) with a message-string fallback for custom Error subclasses.
• What I did not verify: I do not have a live Slack Connect workspace to reproduce against. The classification is based on the error strings reported in #70295 and the Slack API docs. If Slack adds / renames finalize error codes for Connect in the future the allowlist needs to follow.

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: N/A

Risks and Mitigations

• Risk: The allowlist of benign finalize codes hides a real delivery failure if Slack later reuses one of these codes for a case where appended text did NOT reach the user.
  • Mitigation: the three allowlisted codes are recipient-resolution failures that occur at finalize time. chat.appendStream uses the same recipient context, so a user_not_found at stop generally means start/append also hit the same validation path and either already failed (caller sees that as a throw) or succeeded (text is visible). If Slack changes the semantics, the allowlist is a single file and easy to adjust.
• Risk: Non-streaming delivery is accidentally affected.
  • Mitigation: the change is scoped to stopSlackStream inside extensions/slack/src/streaming.ts. Non-streaming delivery paths do not call it.

Changed files

• CHANGELOG.md (modified, +1/-0)
• extensions/slack/src/monitor/message-handler/dispatch.preview-fallback.test.ts (modified, +111/-9)
• extensions/slack/src/monitor/message-handler/dispatch.ts (modified, +86/-4)
• extensions/slack/src/streaming.test.ts (added, +267/-0)
• extensions/slack/src/streaming.ts (modified, +162/-7)