Skip to content

fix(slack): bridge presentation capabilities and renderPresentation through channel outbound facade - #95463

Merged
steipete merged 1 commit into
openclaw:mainfrom
ZOOWH:fix/95440-slack-presentation-complete
Jul 4, 2026
Merged

steipete merged 1 commit into
openclaw:mainfrom
ZOOWH:fix/95440-slack-presentation-complete

Conversation

@ZOOWH

@ZOOWH ZOOWH commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Fixes an issue where Slack users sending portable presentations would receive only plain-text fallback content because the Slack channel facade did not expose its native presentation renderer. This affected Block Kit text, dividers, buttons, selects, card receipts, and callback delivery.

Closes #95440

Why This Change Was Made

The Slack facade now advertises one shared capability contract and lazily delegates native rendering to the Slack outbound adapter. The adapter preserves authored block order and visible payload text, enforces Slack's text/action/option/message limits, falls back without dropping content, and ignores callbacks generated by URL-only buttons (including already-posted legacy buttons). Core receipt classification now recognizes semantic presentations and non-empty rendered presentation blocks before plain text.

User Impact

Slack presentation payloads now render as native Block Kit cards with working controls and card receipts. Unsupported or oversized content remains visible as a complete fallback; ordinary non-presentation media and interactive sends retain their existing behavior.

Evidence

@openclaw-barnacle openclaw-barnacle Bot added channel: slack Channel integration: slack size: S labels Jun 21, 2026
@clawsweeper

clawsweeper Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 4, 2026, 10:47 AM ET / 14:47 UTC.

Summary
The PR bridges Slack presentation capabilities and renderPresentation through the active outbound facade, improves Slack Block Kit rendering and fallback handling, ignores URL-only link callbacks, and updates receipt-classification tests.

PR surface: Source +284, Tests +440. Total +724 across 14 files.

Reproducibility: yes. at source level: current main routes Slack presentation payloads through a facade without renderPresentation and classifies text before presentation, matching the live CLI text-receipt report in #95440.

Review metrics: 2 noteworthy metrics.

  • Slack Facade Presentation Hooks: 2 added. Adding presentationCapabilities and renderPresentation to the registered facade changes the actual Slack delivery path, not just helper code.
  • Shared Receipt Classification: 2 presentation checks promoted before text. The bridge now reports card for portable or rendered presentation payloads before considering plain text, which is an observable receipt contract change.

Stored data model
Persistent data-model change detected: vector/embedding metadata: src/channels/message/outbound-bridge.test.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #95440
Summary: This PR is the current candidate fix for the Slack presentation facade and receipt bug tracked by #95440; earlier overlapping attempts are superseded or only partially related.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster ✨ media proof bonus
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P2] Record maintainer acceptance of card receipts for Slack presentation payloads before merge.

Mantis proof suggestion
A maintainer-run Slack desktop smoke would add useful final-head proof for visible Block Kit rendering and card receipts. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis slack desktop smoke: verify openclaw message send --channel slack --presentation --json posts visible Block Kit and returns receipt kind card.

Risk before merge

  • [P1] Receipt kind changes from text to card for presentation-bearing or rendered-presentation payloads, which is correct for the documented behavior but still observable to downstream consumers.
  • [P2] Slack delivery now exercises native Block Kit rendering, fallback splitting, and URL-only callback ignore paths for more payloads, so the message-delivery semantics need maintainer acceptance.
  • [P1] Live Slack proof was not rerun by the maintainer on the final rewritten head, though contributor screenshots, card-receipt evidence, production confirmation, tests, and exact-head CI cover the same path.

Maintainer options:

  1. Accept Card Receipts For Presentations (recommended)
    Merge after maintainers agree that Slack presentation payloads should report card receipts instead of legacy text receipts.
  2. Preserve Legacy Receipt Semantics
    Change the shared receipt classifier or add an explicit compatibility path if existing consumers must keep seeing text receipts for Slack presentation fallbacks.
  3. Pause For Final-Head Slack Proof
    Hold the PR if maintainers require a fresh maintainer-run Slack Desktop smoke on b0fade6 before accepting the delivery-path change.

Next step before merge

  • [P2] Maintainers should decide whether to accept the observable receipt and Slack delivery semantics; there is no discrete automated repair to queue.

Security
Cleared: The diff changes TypeScript Slack rendering, callback handling, and tests without adding dependencies, workflows, secrets handling, downloads, package scripts, or new execution surfaces.

Review details

Best possible solution:

Land this branch after maintainers accept the card-receipt and Slack delivery semantics; no narrow automated repair is indicated.

Do we have a high-confidence way to reproduce the issue?

Yes at source level: current main routes Slack presentation payloads through a facade without renderPresentation and classifies text before presentation, matching the live CLI text-receipt report in #95440.

Is this the best way to solve the issue?

Yes: delegating the existing Slack renderer through the active facade and classifying rendered presentation data before text is the narrow owner-boundary fix; a second renderer or config switch would duplicate existing contracts.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 445976ed49d0.

Label changes

Label changes:

Label justifications:

Evidence reviewed

PR surface:

Source +284, Tests +440. Total +724 across 14 files.

View PR surface stats
Area Files Added Removed Net
Source 7 377 93 +284
Tests 7 449 9 +440
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 14 826 102 +724

Acceptance criteria:

  • [P1] gh pr checks 95463 --watch=false.
  • [P1] Maintainer-reported focused command: node scripts/run-vitest.mjs src/channels/message/outbound-bridge.test.ts extensions/slack/src/channel.message-adapter.test.ts extensions/slack/src/outbound-adapter.test.ts extensions/slack/src/channel.test.ts extensions/slack/src/outbound-payload.test.ts extensions/slack/src/blocks.test.ts extensions/slack/src/interactive-replies.test.ts extensions/slack/src/shared-interactive.test.ts extensions/slack/src/monitor/events/interactions.test.ts.
  • [P1] Maintainer-reported Testbox check: node scripts/crabbox-wrapper.mjs run -- env OPENCLAW_CHECK_CHANGED_REMOTE_CHILD=1 OPENCLAW_CHANGED_LANES_RAW_SYNC=1 corepack pnpm check:changed.
  • [P1] Maintainer-reported Testbox check: node scripts/crabbox-wrapper.mjs run --shell -- "corepack pnpm check:changed && corepack pnpm build".

What I checked:

  • Current main Slack facade gap: On current main, the registered Slack outbound facade exposes normalizePayload, deliveryCapabilities, and sendPayload but does not expose presentationCapabilities or renderPresentation, so core has no Slack renderer to call on this path. (extensions/slack/src/channel.ts:403, 445976ed49d0)
  • Existing inner Slack renderer: Current main already has presentationCapabilities and renderPresentation in the inner slackOutbound adapter, so the PR bridges an existing renderer rather than inventing a second one. (extensions/slack/src/outbound-adapter.ts:147, 445976ed49d0)
  • Core delivery contract: Core normalizes presentation, adapts it with handler.presentationCapabilities, calls handler.renderPresentation when present, and otherwise falls back to text. (src/infra/outbound/deliver.ts:995, 445976ed49d0)
  • Current main receipt downgrade: Current main classifies nonempty text before presentation, matching the reported text receipt for messages sent with both --message and --presentation. (src/channels/message/outbound-bridge.ts:146, 445976ed49d0)
  • PR facade bridge: At the PR head, slackChannelOutbound exposes SLACK_PRESENTATION_CAPABILITIES and delegates renderPresentation through the lazy runtime outbound delegate. (extensions/slack/src/channel.ts:433, b0fade64cdc6)
  • PR native rendering and fallback path: The PR head renders portable presentations into Slack blocks, preserves visible payload text, and sends separate fallback text when block limits require overflow handling. (extensions/slack/src/outbound-adapter.ts:249, b0fade64cdc6)

Likely related people:

  • steipete: Merged the portable presentation work in [codex] Add presentation capability limits #83032, is assigned to this PR, and authored the current focused PR head that bridges the Slack presentation path. (role: presentation API contributor and recent area contributor; confidence: high; commits: cdd817669a95, 8447f093d740, b0fade64cdc6; files: extensions/slack/src/outbound-adapter.ts, extensions/slack/src/channel.ts, src/infra/outbound/deliver.ts)
  • vincentkoc: Recent merged Slack Block Kit and interactive-message commits overlap the affected Slack rendering and callback surfaces. (role: recent adjacent Slack contributor; confidence: medium; commits: 42efd98ff88b, 12ae4eee7ed1, 54f72214654b; files: extensions/slack/src/channel.ts, extensions/slack/src/blocks-render.ts, extensions/slack/src/reply-blocks.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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 keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (2 earlier review cycles)
  • reviewed 2026-07-03T22:55:47.338Z sha a7711ff72b9017d46aec44150de080098665af1a :: needs maintainer review before merge. :: none
  • reviewed 2026-07-04T14:40:24.939Z sha b0fade6 :: needs maintainer review before merge. :: none

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. labels Jun 21, 2026
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jun 21, 2026
@ZOOWH

ZOOWH commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels Jun 23, 2026
zhangqueping added a commit to zhangqueping/openclaw that referenced this pull request Jun 26, 2026
Match the simpler approach from openclaw#95463: only reorder text↔card
priority in resolvePayloadReceiptKind, removing the over-engineered
hasPresentationChannelData helper and its channelData-specific tests.

The core fix is 3 lines — checking presentation before text so that
payloads with both a presentation and a text fallback correctly
report 'card' instead of 'text'.

Ref: openclaw#95440
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jun 27, 2026
@ZOOWH

ZOOWH commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@ZOOWH

ZOOWH commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Added real Slack proof: openclaw agent --local delivers message to real Slack workspace channel. Bot token verified, presentation bridge active.

@clawsweeper clawsweeper Bot added status: 🔁 re-review loop A fresh ClawSweeper review was explicitly requested after the latest review. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. status: 🔁 re-review loop A fresh ClawSweeper review was explicitly requested after the latest review. labels Jul 2, 2026
@clawsweeper

clawsweeper Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Command router queued. I will update this comment with the next step.

@ZOOWH

ZOOWH commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Changes since last review (a7cde7df):

  1. Slack facade tests (extensions/slack/src/channel.test.ts +50):

    • exposes renderPresentation on the outbound adapter
    • validates presentationCapabilities (supported, buttons, selects, markdownDialect)
    • delegates renderPresentation to slackOutbound and returns presentationBlocks
  2. Outbound-bridge regression (src/channels/message/outbound-bridge.test.ts +16):

    • returns text kind for payloads with only text and no presentation (guards receipt priority reorder)

All 46 Slack + 10 outbound-bridge tests pass.

@clawsweeper

clawsweeper Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added the merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. label Jul 2, 2026
@ZOOWH

ZOOWH commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Added real Slack proof: openclaw message send --channel slack --presentation --json returns receipt.parts[0].kind = "card".

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. and removed status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. labels Jul 3, 2026
@steipete steipete self-assigned this Jul 4, 2026
Co-authored-by: 徐闻涵0668001344 <xu.wenhan1@xydigit.com>
@steipete
steipete force-pushed the fix/95440-slack-presentation-complete branch from a7711ff to b0fade6 Compare July 4, 2026 14:35
@steipete

steipete commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Land-ready verification for b0fade64cdc6c4fce79ae1392b8fa8839cef9d98:

  • Rewrote the contributor branch onto current main as one focused commit; preserved contributor credit with Co-authored-by.

  • Hardened native Slack rendering beyond the original patch: visible payload text, authored block order, verified Slack limits, complete fallback delivery, late legacy-normalization budgeting, safe URL-only actions (including already-posted legacy buttons), and structured receipt precedence.

  • Focused rebased command:

    node scripts/run-vitest.mjs src/channels/message/outbound-bridge.test.ts extensions/slack/src/channel.message-adapter.test.ts extensions/slack/src/outbound-adapter.test.ts extensions/slack/src/channel.test.ts extensions/slack/src/outbound-payload.test.ts extensions/slack/src/blocks.test.ts extensions/slack/src/interactive-replies.test.ts extensions/slack/src/shared-interactive.test.ts extensions/slack/src/monitor/events/interactions.test.ts
    

    Result: 176 tests passed.

  • Fresh autoreview after all fixes: clean, no accepted/actionable findings.

  • node scripts/crabbox-wrapper.mjs run -- env OPENCLAW_CHECK_CHANGED_REMOTE_CHILD=1 OPENCLAW_CHANGED_LANES_RAW_SYNC=1 corepack pnpm check:changed

  • node scripts/crabbox-wrapper.mjs run --shell -- "corepack pnpm check:changed && corepack pnpm build"

  • Live Slack proof was not rerun by the maintainer in this pass. Existing PR/[Bug]: Slack presentation payload never renders Block Kit — delivered as plain text despite capability + interactiveReplies enabled #95440 evidence includes native Block Kit screenshots, receipt.parts[0].kind = "card", and independent production confirmation that this facade bridge restores buttons and callbacks.

Ready to land after exact-head hosted checks.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. labels Jul 4, 2026
@steipete
steipete merged commit f683c9e into openclaw:main Jul 4, 2026
155 of 166 checks passed
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 5, 2026
Co-authored-by: Peter Steinberger <steipete@gmail.com>
@ZOOWH
ZOOWH deleted the fix/95440-slack-presentation-complete branch July 7, 2026 00:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

channel: slack Channel integration: slack merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. P2 Normal backlog priority with limited blast radius. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: L status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Slack presentation payload never renders Block Kit — delivered as plain text despite capability + interactiveReplies enabled

2 participants