fix(slack): warn when channels map is keyed by name instead of channel ID by Alix-007 · Pull Request #89438 · openclaw/openclaw

Alix-007 · 2026-06-02T12:24:19Z

Summary

Slack channel maps are authored as free-form records, but inbound room routing only consumes stable C/G conversation IDs unless dangerous name matching is explicitly enabled. Invalid keys could therefore be accepted while their allow/deny and per-channel overrides were silently ignored.

What changed

Audit effective enabled Slack accounts using the same account enumeration and merge rules as runtime.
Warn for non-routable channel-name and target forms under allowlist policy.
Warn when name-keyed overrides are ignored under open policy, including fail-open deny rules.
Preserve supported lowercase IDs and valid international channel names when dangerous name matching is enabled.
Diagnose misplaced D-prefixed DM IDs and point to the effective account's dmPolicy / allowFrom paths.
Distinguish ambiguous lowercase ID/name forms without claiming valid lowercase IDs are broken.

Routing behavior is unchanged; this is a doctor diagnostic only.

Real behavior proof

Behavior addressed: openclaw doctor now reports Slack channel-map keys that inbound routing cannot consume, including ignored open-policy overrides and misplaced DM IDs.

Real environment tested: Local source checkout at f2ea6c6558aad3f816179c233f770274ff881589.

Exact steps or command run after this patch:

node scripts/run-vitest.mjs extensions/slack/src/doctor.test.ts extensions/slack/src/accounts.test.ts extensions/slack/src/monitor/monitor.test.ts extensions/slack/src/monitor/provider.allowlist.test.ts
pnpm tsgo:extensions
pnpm tsgo:extensions:test
pnpm test:extensions:package-boundary:compile
.agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main

Evidence after fix: 4 test files / 62 tests passed; both extension type lanes passed; all 114 plugin package-boundary compiles passed; final branch-wide autoreview reported no actionable findings.

Observed result after fix: Doctor warnings identify the authored provider/account path, explain wildcard/drop/open-policy outcomes, preserve valid name-matching forms, and route DM remediation to the effective account scope.

What was not tested: No live Slack workspace call. The change is static diagnostics over existing config and routing contracts.

Verification

git diff --check
Post-rebase focused test rerun: 4 files / 62 tests passed

clawsweeper · 2026-06-02T12:25:49Z

Codex review: needs maintainer review before merge. Reviewed June 5, 2026, 3:56 AM ET / 07:56 UTC.

Summary
The PR adds a Slack doctor warning for name-keyed channels.slack.channels entries under effective allowlist policy, plus focused regression tests.

PR surface: Source +57, Tests +117. Total +174 across 2 files.

Reproducibility: yes. Source inspection reproduces the issue shape: current routing ignores name-keyed Slack channel entries unless dangerous name matching is enabled, and current main has no doctor warning for those keys.

Review metrics: none identified.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
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:

none.

Next step before merge

[P2] No repair lane is needed; the remaining action is ordinary maintainer review and merge gating for the current PR head.

Security
Cleared: The diff only changes Slack doctor diagnostics and colocated tests; it does not touch dependencies, workflows, secrets, package metadata, or code-execution surfaces.

Review details

Best possible solution:

Land the Slack-owned doctor warning with the focused tests, while keeping routing ID-first and avoiding a compatibility-changing runtime fallback.

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

Yes. Source inspection reproduces the issue shape: current routing ignores name-keyed Slack channel entries unless dangerous name matching is enabled, and current main has no doctor warning for those keys.

Is this the best way to solve the issue?

Yes. A Slack-owned doctor warning is the best narrow fix because docs already require channel IDs and changing routing or adding strict startup failure would be a larger compatibility decision.

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 1a3ce7c2a8da.

Label changes

Label changes:

add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix live output from the production Slack doctor warning collector across the relevant warning and no-warning cases.

Label justifications:

P2: This is a normal-priority Slack diagnostic bug fix for a silent allowlist misconfiguration with limited blast radius.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes after-fix live output from the production Slack doctor warning collector across the relevant warning and no-warning cases.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix live output from the production Slack doctor warning collector across the relevant warning and no-warning cases.

Evidence reviewed

PR surface:

Source +57, Tests +117. Total +174 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	58	1	+57
Tests	1	117	0	+117
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	175	1	+174

What I checked:

Repository policy read: Root and scoped extension policy were read fully; the review applied the plugin-boundary and PR-review guidance for bundled plugin work. (AGENTS.md:1, 1a3ce7c2a8da)
Scoped extension policy read: extensions/AGENTS.md confirms bundled plugin production code should use openclaw/plugin-sdk/* seams, which the PR does. (extensions/AGENTS.md:1, 1a3ce7c2a8da)
Current main lacks requested warning: Current main's Slack doctor only delegates to the existing mutable-allowlist warning collector and does not inspect channel-map keys for name-keyed Slack channels. (extensions/slack/src/doctor.ts:16, 1a3ce7c2a8da)
Routing source reproduces the silent drop shape: Slack routing only adds channel-name candidates when allowNameMatching is enabled; otherwise a non-ID key without a wildcard returns allowed:false. (extensions/slack/src/monitor/channel-config.ts:85, 1a3ce7c2a8da)
Docs define the expected key shape: Slack docs say channels.slack.channels keys must be stable Slack channel IDs and warn that name-based keys do not match under allowlist routing. Public docs: docs/channels/slack.md. (docs/channels/slack.md:941, 1a3ce7c2a8da)
PR implementation matches the diagnostic boundary: At PR head, the new collector skips dangerous-name-matching scopes, valid Slack IDs, wildcard keys, and non-allowlist effective policies, then appends the warning to the existing Slack doctor collector. (extensions/slack/src/doctor.ts:62, 27d88fa5464b)

Likely related people:

steipete: Recent blame and GitHub path history for Slack doctor, channel-config, provider policy, and Slack docs are dominated by this account, including commits touching the central files and neighboring behavior. (role: recent Slack diagnostics and routing contributor; confidence: high; commits: efd1a9ace6f5, a301df066825, 4785a073d6c0; files: extensions/slack/src/doctor.ts, extensions/slack/src/monitor/channel-config.ts, extensions/slack/src/monitor/provider.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.

Alix-007 · 2026-06-02T12:48:56Z

Thanks for the review! Addressed both rank-up moves so the warning matches routing semantics:

Accept existing valid ID key forms: SLACK_CHANNEL_ID_RE is now case-insensitive and accepts the channel: prefix, so lowercase IDs and channel:<id> keys are no longer flagged.
Resolve inherited groupPolicy: an account without its own groupPolicy now inherits the provider-level policy, so name-keyed channels in accounts under an inherited open policy are not flagged.

Added regression tests for both (lowercase / channel:-prefixed ID keys not flagged; account inheriting an open policy not flagged) — doctor.test.ts now 8 passing. Verified locally: tsgo:extensions + tsgo:extensions:test clean, oxlint clean. Current head c150a9e358. @clawsweeper re-review

clawsweeper · 2026-06-02T12:49:00Z

🦞👀
ClawSweeper picked this up.

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

Alix-007 · 2026-06-02T13:43:32Z

Addressed the 4th P1 (effective group policy): the doctor now computes the Slack group policy the same way the runtime does — resolveOpenProviderRuntimeGroupPolicy + resolveDefaultGroupPolicy — instead of defaulting a policy-omitted provider to allowlist. So a configured Slack block that omits groupPolicy resolves to open (no warning, matching runtime), and channels.defaults.groupPolicy is honored. Added regression tests: omitted policy ⇒ open/no-warn; channels.defaults allowlist ⇒ warn, open ⇒ no-warn. doctor.test.ts now 10 passing; tsgo:extensions + :test clean (only a pre-existing discord error untouched by this PR), oxlint clean. Current head 30bb9e8b9f. @clawsweeper re-review

clawsweeper · 2026-06-02T13:43:36Z

🦞👀
ClawSweeper picked this up.

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

Alix-007 · 2026-06-02T14:50:09Z

Aligned the effective-policy check with the loaded Slack default per the Best-possible-solution: an omitted/default groupPolicy is treated as allowlist (the loaded Slack default — setup-core uses groupPolicy ?? "allowlist"), so the issue's default config now warns; explicit open/disabled are skipped, channels.defaults.groupPolicy is honored, and name-matching-enabled accounts / valid ID keys (case-insensitive + channel: prefix) stay unflagged. Updated the omitted-policy regression test to expect the warning, kept the channels.defaults allowlist/open + lowercase/channel: ID + inherited-open cases. doctor.test.ts green; tsgo:extensions + :test clean (only the pre-existing discord error untouched by this PR), oxlint clean. Current head 99ddf10cde. @clawsweeper re-review

clawsweeper · 2026-06-02T14:50:12Z

🦞👀
ClawSweeper picked this up.

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

…l ID Under groupPolicy: "allowlist" (the default), a `channels.slack.channels` entry keyed by a human channel NAME instead of a Slack channel ID never matches the ID-first routing, so messages in that channel are silently dropped — the schema accepts the shape and no diagnostic fires. `slackDoctor.collectMutableAllowlistWarnings` now also flags name-keyed channel map entries (skipping valid IDs, the "*" wildcard, accounts with name matching enabled, and non-allowlist group policies), telling the operator to re-key with the channel's ID. Diagnostic-only — routing semantics are unchanged. Fixes openclaw#81665. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

Co-authored-by: Alix-007 <[email protected]>

steipete · 2026-06-13T10:13:09Z

Land-ready verification for f2ea6c6558aad3f816179c233f770274ff881589.

Local proof:

node scripts/run-vitest.mjs extensions/slack/src/doctor.test.ts extensions/slack/src/accounts.test.ts extensions/slack/src/monitor/monitor.test.ts extensions/slack/src/monitor/provider.allowlist.test.ts — 4 files, 62 tests passed
pnpm tsgo:extensions — passed
pnpm tsgo:extensions:test — passed
pnpm test:extensions:package-boundary:compile — 114 plugins passed
git diff --check — passed
Fresh branch autoreview — no actionable findings, confidence 0.96

GitHub proof:

CI run 27463772708 — passed
CodeQL Critical Quality run 27463772706 — passed
Real behavior proof run 27463784993 — passed

Known proof gap: no live Slack workspace was used. The change is a static doctor diagnostic, covered against the current Slack account-merging and inbound routing contracts.

Diagnose Slack channel-map keys that cannot route as configured, including account inheritance, open-policy overrides, malformed room identifiers, and DM identifiers. Fixes openclaw#81665 Co-authored-by: Alix-007 <[email protected]>

openclaw-barnacle Bot added channel: slack Channel integration: slack size: S proof: supplied External PR includes structured after-fix real behavior proof. labels Jun 2, 2026

Alix-007 force-pushed the fix/slack-name-keyed-channel-81665 branch from ce85dad to c150a9e Compare June 2, 2026 12:48

openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 2, 2026

clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 2, 2026

Alix-007 force-pushed the fix/slack-name-keyed-channel-81665 branch from c150a9e to 30bb9e8 Compare June 2, 2026 13:42

openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 2, 2026

Alix-007 force-pushed the fix/slack-name-keyed-channel-81665 branch from 30bb9e8 to 99ddf10 Compare June 2, 2026 14:49

openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 2, 2026

clawsweeper Bot added the status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. label Jun 2, 2026

openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 5, 2026

clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 5, 2026

Alix-007 and others added 11 commits June 13, 2026 11:08

fix(slack): use scoped plugin sdk config imports

bd36426

test(slack): consolidate channel key warning coverage

825cfbb

Co-authored-by: Alix-007 <[email protected]>

fix(slack): tighten channel key diagnostics

b2077db

Co-authored-by: Alix-007 <[email protected]>

fix(slack): cover inherited channel diagnostics

9f34085

Co-authored-by: Alix-007 <[email protected]>

fix(slack): align channel key diagnostics

2fa59c2

Co-authored-by: Alix-007 <[email protected]>

fix(slack): audit effective channel scopes

ed38027

Co-authored-by: Alix-007 <[email protected]>

docs(slack): explain authored channel key diagnostics

cf86bc4

Co-authored-by: Alix-007 <[email protected]>

fix(slack): validate name-matching channel keys

4ed03d3

Co-authored-by: Alix-007 <[email protected]>

docs(slack): record international name contract

e02eaa1

Co-authored-by: Alix-007 <[email protected]>

fix(slack): diagnose DM IDs in channel maps

f2ea6c6

Co-authored-by: Alix-007 <[email protected]>

steipete force-pushed the fix/slack-name-keyed-channel-81665 branch from 27d88fa to f2ea6c6 Compare June 13, 2026 10:08

openclaw-barnacle Bot added size: M and removed size: S proof: sufficient ClawSweeper judged the real behavior proof convincing. labels Jun 13, 2026

steipete merged commit 26281a8 into openclaw:main Jun 13, 2026
172 of 175 checks passed

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

📡 Upstream Digest — 2026-06-13 10:22 UTC curtismercier/openclaw-mods#1065

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(slack): warn when channels map is keyed by name instead of channel ID#89438

fix(slack): warn when channels map is keyed by name instead of channel ID#89438
steipete merged 11 commits into
openclaw:mainfrom
Alix-007:fix/slack-name-keyed-channel-81665

Alix-007 commented Jun 2, 2026 •

edited by steipete

Loading

Uh oh!

clawsweeper Bot commented Jun 2, 2026 •

edited

Loading

Uh oh!

Alix-007 commented Jun 2, 2026

Uh oh!

clawsweeper Bot commented Jun 2, 2026

Uh oh!

Alix-007 commented Jun 2, 2026

Uh oh!

clawsweeper Bot commented Jun 2, 2026

Uh oh!

Alix-007 commented Jun 2, 2026

Uh oh!

clawsweeper Bot commented Jun 2, 2026

Uh oh!

steipete commented Jun 13, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

Alix-007 commented Jun 2, 2026 • edited by steipete Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What changed

Real behavior proof

Verification

Uh oh!

clawsweeper Bot commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Alix-007 commented Jun 2, 2026

Uh oh!

clawsweeper Bot commented Jun 2, 2026

Uh oh!

Alix-007 commented Jun 2, 2026

Uh oh!

clawsweeper Bot commented Jun 2, 2026

Uh oh!

Alix-007 commented Jun 2, 2026

Uh oh!

clawsweeper Bot commented Jun 2, 2026

Uh oh!

steipete commented Jun 13, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Alix-007 commented Jun 2, 2026 •

edited by steipete

Loading

clawsweeper Bot commented Jun 2, 2026 •

edited

Loading