[Feature]: Channel Broker Phase 2D Slack opt-in broker routing proof#86157
[Feature]: Channel Broker Phase 2D Slack opt-in broker routing proof#86157100yenadmin wants to merge 71 commits into
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed May 27, 2026, 7:38 AM ET / 11:38 UTC. Summary PR surface: Source +3430, Tests +4694, Docs +306, Config +77, Generated 0, Other +22. Total +8529 across 57 files. Reproducibility: yes. for the review finding: source inspection shows accessGroup allowFrom entries return true at the HTTP gate before sender membership is checked. I did not run a live webhook because this review is read-only. Review metrics: 2 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance: Risk before merge
Maintainer options:
Next step before merge Security Review findings
Review detailsBest possible solution: Fix broker webhook access-group admission, refresh current-head real behavior proof, and land the broker phases only after maintainers approve the public SDK, config, and security contract. Do we have a high-confidence way to reproduce the issue? Yes for the review finding: source inspection shows accessGroup allowFrom entries return true at the HTTP gate before sender membership is checked. I did not run a live webhook because this review is read-only. Is this the best way to solve the issue? No, not yet. The broker direction may be useful, but this PR should not merge until the authorization gate is fixed, proof is refreshed for the current head, and maintainers approve the public SDK/config rollout. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against 0503853c294b. Label changesLabel justifications:
Evidence reviewedPR surface: Source +3430, Tests +4694, Docs +306, Config +77, Generated 0, Other +22. Total +8529 across 57 files. View PR surface stats
Security concerns:
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 🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat. Where did the egg go?
|
12249be to
ae4dc51
Compare
ae4dc51 to
47b7928
Compare
|
No dependency changes detected. Learn more about Socket for GitHub. 👍 No dependency changes detected in pull request |
Dependency Changes DetectedThis PR changes dependency-related files. Maintainers should confirm these changes are intentional. Changed files:
Maintainer follow-up:
|
47b7928 to
3bf11d6
Compare
Add Vincent Koc as a co-author for the PR context and review trail. Co-authored-by: Vincent Koc <vincentkoc@ieee.org>
Add Vincent Koc as a co-author for the PR context and review trail. Co-authored-by: Vincent Koc <vincentkoc@ieee.org>
Add Vincent Koc as a co-author for the PR context and review trail. Co-authored-by: Vincent Koc <vincentkoc@ieee.org>
Add Vincent Koc as a co-author for the PR context and review trail. Co-authored-by: Vincent Koc <vincentkoc@ieee.org>
This comment was marked as spam.
This comment was marked as spam.
|
Hi Eva, Thank you for putting this together and for the amount of work that clearly went into the Channel Broker stack. I'm sorry to close it after that investment, but we are not going to accept this PR stack. The changes reach too deeply across OpenClaw's channel, plugin SDK, runtime, config, and security boundaries to land without prior maintainer consultation and an agreed design plan. This needs to start as a smaller design discussion with maintainer-approved boundaries before code of this scope can be considered. I'm closing this PR and the linked phase stack for the same reason. Thank you again for the contribution. |
Maintainer TL;DR
This stack consolidates the recurring Telegram/Discord/Slack/WhatsApp/Signal/iMessage/etc. maintenance churn behind one Channel Broker contract. OpenClaw keeps the shared semantics that repeatedly regress across native channel plugins: sessions, allowlists, routing, /verbose, streaming policy, durable final sends, receipts, retries, and auditability. Broker providers own platform mechanics: bot APIs, bridge/device state, native ids, delivery quirks, and provider capabilities.
This PR proves Slack DMs and channel threads can route through broker-owned semantics while native Slack remains the default path.
The direction is intentionally incremental: first land the versioned broker SDK/plugin foundation, then prove conformance, then prove opt-in routing for the major channels, then model long-tail and constrained providers. Only after migration paths exist should legacy native-channel feature work freeze in #86115.
Why This Is Better
Today, one fix to streaming, /verbose, or delivery behavior can make Telegram fast this week and then break Discord, Slack, or another native plugin the next week. The broker path makes those shared semantics one OpenClaw-owned contract instead of many platform-specific re-implementations. When a provider adds support for previews, threads, receipts, media, or constrained account modes, that capability can become available through the same contract to routed channels whose provider declares and passes conformance for it.
Stack Shape
Ownership Model
Review Shape
These PRs are intentionally linear. GitHub still shows
mainas the base for each PR so maintainers can land them one by one from fork branches; the branches themselves are stacked, and maintainers can review the focused adjacent diffs here:This PR
7614f3bcfbCurrent Review State
This PR is live for maintainer review. Current head:
7614f3bcfbThis branch also carries the tiny
src/agents/model-catalog-visibility.test.tstimeout hardening inherited from #86096 after the unrelatedchecks-node-agentic-agentsshard repeatedly timed out there while the single file passed locally.Focused local validation after the latest package/deadcode/doc/capability repair and upstream/main rebase ran from
/Volumes/LEXAR/repos/openclaw-channel-broker:git diff --check upstream/main..HEADpnpm exec vitest run src/plugin-sdk/channel-broker.test.ts --config test/vitest/vitest.unit-fast.config.ts --maxWorkers=1pnpm exec vitest run extensions/channel-broker/src/channel.test.ts extensions/channel-broker/src/config-schema.test.ts extensions/channel-broker/src/conformance.test.ts extensions/channel-broker/src/http-routes.test.ts --config test/vitest/vitest.extension-channel-broker.config.ts --maxWorkers=1pnpm exec vitest run src/plugins/contracts/plugin-sdk-package-contract-guardrails.test.ts --config test/vitest/vitest.contracts-plugin.config.ts --maxWorkers=1pnpm exec vitest run test/scripts/bundled-plugin-build-entries.test.ts --config test/vitest/vitest.tooling.config.ts --maxWorkers=1pnpm exec vitest run test/scripts/check-deadcode-unused-files.test.ts --config test/vitest/vitest.unit-fast.config.ts --maxWorkers=1node scripts/check-deadcode-unused-files.mjspnpm exec tsc -p extensions/channel-broker/tsconfig.json --noEmitBroad validation remains owned by GitHub Actions.
Summary
Closes #86112
Related #86108
Related #86105
Supersedes closed PR #86142, which was closed by the repository active-PR cap rather than by an implementation failure.
What changed
broker:slack:user:U12345678into broker provider requests withconversation.type = "direct".broker:slack:channel:C12345678?threadId=1716500000.000001into broker provider requests withconversation.type = "channel"andthreadId.slack:direct%3A<user>and thread routes asslack:<channel>?threadId=<threadTs>.Real Behavior Proof
channel-brokerwhile preserving direct/channel conversation semantics and SlackthreadTsas brokerthreadId./Volumes/LEXAR/repos/openclaw-channel-broker, using the Channel Broker plugin and SDK from the stacked rollout branch on May 24, 2026.node --import tsx --input-type=moduleterminal proof that calledchannelBrokerPlugin.message.send.textwithto: "broker:slack:channel:C12345678?threadId=1716500000.000001"and captured the provider request passed to the configured broker runtime.{ "label": "slack-thread", "messageId": "native-slack-thread", "platform": "slack", "conversation": { "id": "C12345678", "type": "channel", "threadId": "1716500000.000001" }, "requirements": { "text": true, "thread": true } }threadIdfield.Focused diff for this stack layer:
100yenadmin/openclaw@feat/channel-broker-discord-routing...feat/channel-broker-slack-routing
Validation
Run locally from
/Volumes/LEXAR/repos/openclaw-channel-broker:pnpm exec vitest run extensions/channel-broker/src/channel.test.ts extensions/channel-broker/src/conformance.test.ts --config test/vitest/vitest.extension-channel-broker.config.tspnpm exec tsc --noEmit -p extensions/channel-broker/tsconfig.jsongit diff --checkpnpm exec oxfmt --check extensions/channel-broker/src/target.ts extensions/channel-broker/src/channel.ts extensions/channel-broker/src/channel.test.tspnpm exec oxlint extensions/channel-broker --format unixReview state