fix(mattermost): handle missing channel types by eleqtrizit · Pull Request #78530 · openclaw/openclaw

eleqtrizit · 2026-05-06T15:33:45Z

Summary

Handles missing Mattermost channel type metadata without defaulting to public-channel behavior.
Keeps normal direct, group, private, and public channel type handling unchanged when metadata is present.

Changes

Added a shared Mattermost channel type resolver and made missing or blank channel types reject explicitly.
Dropped or denied Mattermost message, button, reaction, and command authorization paths when channel type metadata cannot be resolved.
Added regression coverage for missing channel types, incomplete channel records, and the reported post/button runtime paths.
Stabilized the regression tests so async assertions are awaited and drop-path monitor tests shut down cleanly after proving no dispatch occurs.

Validation

corepack pnpm install
corepack pnpm exec tsx -e 'import { mapMattermostChannelTypeToChatType, resolveMattermostTrustedChatKind } from "./extensions/mattermost/src/mattermost/monitor-gating.ts"; ...'
node scripts/run-vitest.mjs extensions/mattermost/src/mattermost/monitor-auth.test.ts
node scripts/run-vitest.mjs extensions/mattermost/src/mattermost/monitor-gating.test.ts extensions/mattermost/src/mattermost/monitor.channel-kind.test.ts
node scripts/run-vitest.mjs extensions/mattermost/src/mattermost/monitor.inbound-system-event.test.ts
corepack pnpm exec oxfmt --check --threads=1 extensions/mattermost/src/mattermost/monitor-auth.test.ts extensions/mattermost/src/mattermost/monitor.inbound-system-event.test.ts
git diff --check origin/main...HEAD

Real behavior proof

Behavior addressed: Missing or blank Mattermost channel type metadata should fail closed instead of mapping to public-channel handling; valid direct, group, private, and public channel codes should still map normally.

Real environment tested: Local OpenClaw checkout on branch 571 at 505332585a, Node v22.22.2, pnpm 11.1.0.

Exact steps or command run after this patch: Ran a local terminal command with corepack pnpm exec tsx -e ... importing the actual Mattermost channel-type module from extensions/mattermost/src/mattermost/monitor-gating.ts, then ran the targeted Mattermost Vitest files through node scripts/run-vitest.mjs for the shared mapper/auth helpers and monitor-level inbound post/button regression paths listed above.

Evidence after fix:

map rejects undefined Mattermost channel type is required
map rejects null Mattermost channel type is required
map rejects " " Mattermost channel type is required
trusted rejects missing type Mattermost channel type is required
direct direct
group group
channel channel

Targeted regression tests also passed: monitor-auth.test.ts passed 4 tests; monitor-gating.test.ts plus monitor.channel-kind.test.ts passed 11 tests; monitor.inbound-system-event.test.ts passed 4 tests, including the missing channel type post drop and button dispatch skip cases.

Observed result after fix: Missing channel type metadata now rejects or drops before authorization/routing can treat the event as channel, while known Mattermost type codes continue to route as before.

What was not tested: A live Mattermost server callback round trip was not run locally.

Notes

No changelog entry per maintainer direction for this security-fix branch.

clawsweeper · 2026-05-06T15:37:31Z

Codex review: needs maintainer review before merge.

Workflow note: Future ClawSweeper reviews update this same comment in place.

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.

Summary
The PR adds a Mattermost channel-type resolver, rejects missing or blank channel type metadata across auth, post, button, and reaction paths, adds regression tests, and records a changelog fix.

Reproducibility: yes. source inspection gives a high-confidence reproduction path: current main maps missing Mattermost channel type values to channel, and the post/button call sites pass channelInfo?.type into that fallback. I did not run tests because this review is read-only.

PR rating
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Summary: Focused plugin-owned fix with sufficient terminal proof and no blocking findings, with normal maintainer review needed for the intentional delivery compatibility change.

Rank-up moves:

Confirm the fail-closed Mattermost behavior is the intended compatibility tradeoff before merge.

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.

PR egg
✨ Hatched: 🥚 common Sunspot Patch Peep

        .--^^^^--.           
     .-'  o    o  '-.        
    /       \__/      \      
   |    /\  ____  /\   |     
   |   /  \/____\/  \  |     
    \  \_.------._/  /       
     '._  `----'  _.'        
        '-.____.-'           
       _/|_|  |_|\_          
      /__|      |__\         
       .-----------.         
      '-------------'

Rarity: 🥚 common.
Trait: stacks clean commits.
Share on X: post this hatch
Copy: My PR egg hatched a 🥚 common Sunspot Patch Peep in ClawSweeper.

What is this egg doing here?

Eggs appear after the PR passes real-behavior proof. It is here for vibes, not verdicts: it does not change labels, ratings, merge decisions, or automation.
The shell reacts to review momentum: open follow-up work warms it up, re-review makes it wobble, and a clean final review lets it hatch.
How to hatch it: reach status: 👀 ready for maintainer look or status: 🚀 automerge armed; that usually means sufficient real-behavior proof, no blocking P0/P1/P2 findings, no security attention needed, and clean correctness.
The hatch is seeded from this repository and PR number, so the same PR keeps the same creature; the reviewed head SHA can only change safe visual details.
Rarity is just collectible sparkle: 🥚 common, 🌱 uncommon, 💎 rare, ✨ glimmer, and 🌈 legendary.

Real behavior proof
Sufficient (terminal): The PR body includes after-fix terminal output from the actual Mattermost resolver plus targeted Mattermost regression test results, and the latest head only adds test typing and changelog changes.

Risk before merge
Why this matters: - Merging intentionally changes malformed or lookup-failed Mattermost events from public-channel fallback to drop/deny behavior, so any setup relying on that fallback would stop receiving those messages, reactions, or button dispatches.

No live Mattermost callback/server round trip is attached; the available proof is terminal output against the actual module plus targeted monitor regression tests.

Maintainer options:

Accept fail-closed Mattermost handling (recommended)
Land after maintainers confirm unresolved Mattermost channel metadata should be denied or dropped instead of routed as a public channel.
Require live Mattermost proof first
Ask for or run a live Mattermost callback/post scenario if transport-level evidence is required before accepting the delivery behavior change.
Pause if compatibility should win
Hold or close the PR if maintainers decide the existing malformed-event public-channel fallback must remain the default.

Next step before merge
The protected maintainer label and intentional fail-closed compatibility/message-delivery change require human maintainer acceptance; I found no narrow automated repair to request.

Security
Cleared: The diff strengthens Mattermost fail-closed routing/auth behavior and only touches plugin source, tests, and changelog; I found no new supply-chain, secret, permission, or execution-path concern.

Review details

Best possible solution:

Land the Mattermost plugin-owned fail-closed fix once maintainers accept the malformed-event compatibility tradeoff, keeping the focused regression coverage with it.

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

Yes, source inspection gives a high-confidence reproduction path: current main maps missing Mattermost channel type values to channel, and the post/button call sites pass channelInfo?.type into that fallback. I did not run tests because this review is read-only.

Is this the best way to solve the issue?

Yes, mostly: the PR keeps the fix inside the Mattermost plugin boundary and centralizes channel-type resolution instead of changing core contracts. The remaining question is not code shape but maintainer acceptance of fail-closed drops/denials for unresolved metadata.

Label justifications:

P1: The PR addresses a Mattermost routing/auth regression where missing channel metadata can be treated as public-channel traffic.
merge-risk: 🚨 compatibility: Existing malformed or lookup-failed events that previously flowed through the public-channel fallback will be denied or dropped after merge.
merge-risk: 🚨 message-delivery: The changed post, reaction, and button paths can suppress delivery when Mattermost channel type metadata cannot be resolved.

What I checked:

Current main fallback: On current main, mapMattermostChannelTypeToChatType returns channel when channelType is falsy, and existing tests assert undefined maps to channel. (extensions/mattermost/src/mattermost/monitor-gating.ts:3, 1fb09069c342)
Affected runtime paths: The button system-event/session-key path, synthetic button dispatch path, and post handler all consume channelInfo?.type through the fallback mapper/trusted resolver on current main. (extensions/mattermost/src/mattermost/monitor.ts:669, 1fb09069c342)
Existing fail-closed precedent: The current reaction path already drops when it cannot resolve channelInfo.type, which supports the PR's fail-closed direction for security-sensitive routing. (extensions/mattermost/src/mattermost/monitor.ts:1984, 1fb09069c342)
PR diff shape: The PR patch adds resolveMattermostChannelType, makes the mapper throw on missing/blank values, and updates auth/post/button/reaction paths to deny or drop when no trusted channel type resolves. (extensions/mattermost/src/mattermost/monitor-gating.ts:3, 420471125332)
Real behavior proof in PR body: The PR body includes after-fix terminal output from importing the actual Mattermost channel-type module and targeted Mattermost regression test results; the later head changes are test typing and changelog only. (420471125332)
Related Mattermost context: The related closed unmerged PR fix(mattermost): return 'unknown' ChatType when channel type is not resolvable #57970 described the same missing-channel-type-to-public-channel fallback, and the open Mattermost threading PR fix(mattermost): route thread replies correctly when replyToMode is off #57565 depended on that behavior being fixed.

Likely related people:

Yao: Current-main blame for the central Mattermost mapper and relevant monitor call sites points to commit 6a5a1353c7f04d4ca41ace35269da7280292c120. (role: recent area contributor; confidence: medium; commits: 6a5a1353c7f0; files: extensions/mattermost/src/mattermost/monitor-gating.ts, extensions/mattermost/src/mattermost/monitor.ts, extensions/mattermost/src/mattermost/monitor-auth.ts)
dave: Authored the related missing-channel-type proposal and the dependent Mattermost threading PR referenced in this PR's timeline, so they have useful context even though that work is not merged on current main. (role: adjacent bug-fix proposer; confidence: medium; commits: 3779f1536027, 02de7789bcfb; files: extensions/mattermost/src/mattermost/monitor-gating.ts, extensions/mattermost/src/mattermost/monitor.ts)

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

eleqtrizit · 2026-05-06T17:41:10Z

Standards check update for this PR:

Confirmed the branch diff is scoped to the Mattermost plugin and keeps the fix inside the owning extension boundary; no core, SDK, protocol, manifest, or config surface changes.
Re-reviewed the issue/advisory behavior and kept the security posture fail-closed: missing/blank Mattermost channel type data now rejects instead of falling back to the less restrictive channel classification.
Addressed review feedback by documenting the exported helper contract, explaining the channel-type source ordering in the handler, and adding PoC parity coverage for missing inputs.
Added monitor-level regression coverage for the reported runtime paths: missing post channel type with failed channel lookup does not dispatch, and later button channel lookup failure skips both system-event enqueue and synthetic dispatch.
Rechecked commit scope before push: pushed d9e970eb28 on branch 571; committed files are limited to Mattermost source/tests.
Validation run: corepack pnpm exec oxfmt --write --threads=1 extensions/mattermost/src/mattermost/monitor-gating.ts extensions/mattermost/src/mattermost/monitor.ts extensions/mattermost/src/mattermost/monitor.channel-kind.test.ts extensions/mattermost/src/mattermost/monitor.inbound-system-event.test.ts, corepack pnpm exec oxfmt --check --threads=1 extensions/mattermost/src/mattermost/monitor-gating.ts extensions/mattermost/src/mattermost/monitor.ts extensions/mattermost/src/mattermost/monitor.channel-kind.test.ts extensions/mattermost/src/mattermost/monitor.inbound-system-event.test.ts, git diff --check, and a tsx helper smoke for reject/known-code behavior. The targeted Mattermost Vitest command was attempted after corepack pnpm install, but this checkout still fails before test collection on Cannot find package '@openclaw/fs-safe/config' imported from src/infra/fs-safe-defaults.ts.

* fix(browser): guard current tab act routes * fix(browser): document current-tab route guard

…law#83707) Summary: - The PR adds raw explicit-port detection for browser CDP URLs, updates profile resolution precedence, adds regression tests, and records the browser fix in the changelog. - Reproducibility: yes. Source inspection shows current main resolves a portless profile `cdpUrl` through `par ... 443, and overwrites the configured `cdpPort`; the source PR also provides live before/after Chrome output. Automerge notes: - PR branch already contained follow-up commit before automerge: fix(browser): encapsulate explicit-port detection in parseBrowserHttpUrl - PR branch already contained follow-up commit before automerge: fix(browser): preserve explicit cdpPort when cdpUrl omits port Validation: - ClawSweeper review passed for head 070c31c. - Required merge gates passed before the squash merge. Prepared head SHA: 070c31c Review: openclaw#83707 (comment) Co-authored-by: Hongwei Ma <[email protected]> Co-authored-by: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com> Co-authored-by: clawsweeper[bot] <274271284+clawsweeper[bot]@users.noreply.github.com> Approved-by: takhoffman Co-authored-by: takhoffman <[email protected]>

Summary: - The PR changes collect-mode follow-up queue routing so unresolved-origin items can batch with a single resolved route and later compatible items can resume batching after a true cross-channel drain. - Reproducibility: yes. at source level: current main treats unkeyed-plus-same-keyed queue items as cross-chan ... failing path is directly visible in `src/utils/queue-helpers.ts` and `src/auto-reply/reply/queue/drain.ts`. Automerge notes: - PR branch already contained follow-up commit before automerge: Merge remote-tracking branch 'origin/main' into maint-83701-20260518 Validation: - ClawSweeper review passed for head e6ad029. - Required merge gates passed before the squash merge. Prepared head SHA: e6ad029 Review: openclaw#83701 (comment) Co-authored-by: Andy Ye <[email protected]> Co-authored-by: clawsweeper[bot] <274271284+clawsweeper[bot]@users.noreply.github.com> Approved-by: takhoffman Co-authored-by: takhoffman <[email protected]>

…#83726) Summary: - The PR passes the effective OpenAI/Codex auth provider set into `/models` provider-header labeling, adds focused regression tests, and records the user-facing fix in the changelog. - Reproducibility: yes. Current main lacks `acceptedProviderIds` in the shared picker header path, and the source PR's Mantis baseline/candidate proof shows the visible Telegram header mismatch and after-fix OAuth label. Automerge notes: - PR branch already contained follow-up commit before automerge: fix(models): label picker auth via effective provider order Validation: - ClawSweeper review passed for head 8ca2924. - Required merge gates passed before the squash merge. Prepared head SHA: 8ca2924 Review: openclaw#83726 (comment) Co-authored-by: Stellar鱼 <[email protected]> Co-authored-by: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com> Co-authored-by: clawsweeper[bot] <274271284+clawsweeper[bot]@users.noreply.github.com> Approved-by: takhoffman Co-authored-by: takhoffman <[email protected]>

…ns (openclaw#70110) (openclaw#72348) Summary: - This PR adds and registers a Nextcloud Talk message action adapter for add-only reactions, updates reaction docs, and adds adapter plus sender tests. - Reproducibility: yes. Source inspection on current main shows Nextcloud Talk advertises reactions and has a ... ion sender, but the plugin lacks `actions.handleAction`, so shared `react` dispatch has no channel handler. Automerge notes: - PR branch already contained follow-up commit before automerge: test(nextcloud-talk): cover reaction sender request path - PR branch already contained follow-up commit before automerge: fix(nextcloud-talk): harden react null-guard; fix disabled-account te… - PR branch already contained follow-up commit before automerge: fix(nextcloud-talk): reject react remove requests instead of silently… - PR branch already contained follow-up commit before automerge: fix(nextcloud-talk): inline listEnabledAccounts helper after main cle… - PR branch already contained follow-up commit before automerge: docs(nextcloud-talk): note add-only react support in reactions and me… Validation: - ClawSweeper review passed for head 9817fed. - Required merge gates passed before the squash merge. Prepared head SHA: 9817fed Review: openclaw#72348 (comment) Co-authored-by: NW <[email protected]> Co-authored-by: clawsweeper[bot] <274271284+clawsweeper[bot]@users.noreply.github.com> Approved-by: takhoffman Co-authored-by: takhoffman <[email protected]>

…83717) Summary: - Adds a personal-agent QA-Lab share-safe diagnostics scenario with mock-openai support, pack registration/tests, docs, and changelog coverage. - Reproducibility: not applicable. This PR adds a new QA-Lab scenario rather than fixing a current-main bug. T ... ce PR provides a clear after-patch validation path using qa-channel, a real gateway child, and mock-openai. Automerge notes: - No ClawSweeper repair was needed after automerge opt-in. Validation: - ClawSweeper review passed for head 46eb0af. - Required merge gates passed before the squash merge. Prepared head SHA: 46eb0af Review: openclaw#83717 (comment) Co-authored-by: Firas Alswihry <[email protected]> Co-authored-by: clawsweeper[bot] <274271284+clawsweeper[bot]@users.noreply.github.com> Approved-by: takhoffman Co-authored-by: takhoffman <[email protected]>

openclaw-barnacle Bot added channel: mattermost Channel integration: mattermost size: S maintainer Maintainer-authored PR labels May 6, 2026

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

openclaw-barnacle Bot added size: M and removed size: S labels May 6, 2026

This was referenced May 11, 2026

fix(mattermost): return 'unknown' ChatType when channel type is not resolvable #57970

Closed

fix(mattermost): route thread replies correctly when replyToMode is off #57565

Closed

eleqtrizit force-pushed the 571 branch 2 times, most recently from 905ff99 to 5053325 Compare May 17, 2026 15:09

eleqtrizit added 4 commits May 18, 2026 11:21

fix(mattermost): fail closed on missing channel types

ed6587f

fix(mattermost): pin channel type fail-closed tests

9f7899f

fix(mattermost): stabilize channel type regression tests

0d99823

fix(mattermost): type fake interaction request

5aa9798

eleqtrizit force-pushed the 571 branch from 5053325 to 5aa9798 Compare May 18, 2026 18:45

fix(mattermost): record fail-closed changelog

4204711

eleqtrizit self-assigned this May 18, 2026

eleqtrizit and others added 8 commits May 18, 2026 21:32

fix(browser): guard current tab act routes (openclaw#78523)

eac8263

* fix(browser): guard current tab act routes * fix(browser): document current-tab route guard

fix(doctor): anchor WhatsApp TUI process matching (openclaw#83313)

6c23682

fix(mattermost): record fail-closed changelog

08f6684

eleqtrizit requested a review from a team as a code owner May 18, 2026 21:34

openclaw-barnacle Bot added docs Improvements or additions to documentation channel: nextcloud-talk Channel integration: nextcloud-talk commands Command implementations agents Agent runtime and tooling extensions: qa-lab size: XL and removed size: M labels May 18, 2026

eleqtrizit closed this May 18, 2026

eleqtrizit deleted the 571 branch May 18, 2026 21:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(mattermost): handle missing channel types#78530

fix(mattermost): handle missing channel types#78530
eleqtrizit wants to merge 13 commits into
openclaw:mainfrom
eleqtrizit:571

eleqtrizit commented May 6, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented May 6, 2026 •

edited

Loading

Uh oh!

eleqtrizit commented May 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Uh oh!

Conversation

eleqtrizit commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Validation

Real behavior proof

Notes

Uh oh!

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

Uh oh!

Uh oh!

eleqtrizit commented May 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

eleqtrizit commented May 6, 2026 •

edited

Loading

clawsweeper Bot commented May 6, 2026 •

edited

Loading