fix(mattermost): tag typed text-slash control commands so /new and /reset acks survive message_tool_only suppression by amknight · Pull Request #86825 · openclaw/openclaw

amknight · 2026-05-26T08:42:13Z

Summary

Closes #86664 (the beta-blocker that PR #86767 attempted but did not actually fix — see closeout comment on #86767).

In Mattermost direct chats (and any Mattermost channel) running under a message_tool_only source-reply delivery mode — most commonly the Codex harness default (extensions/codex/harness.ts:41) — typed control commands like /new, /reset, /new (leading-space to bypass Mattermost's native slash UI), and the ACP/soft-reset variants silently changed session state because their acknowledgement replies were suppressed.

Root cause: the Mattermost text-post inbound handler (extensions/mattermost/src/mattermost/monitor.ts:1573-1611) already sets CommandAuthorized: commandAuthorized for typed control commands, but never set CommandSource. Without CommandSource: "text", resolveCommandTurnContext (src/auto-reply/command-turn-context.ts:159-176) defaults the turn kind to "normal" and forces authorized: false, so isExplicitCommandTurn returns false and the existing escape hatch in source-reply-delivery-mode.ts:54-56 (if (isExplicitSourceReplyCommand(ctx)) return "automatic") never fires — message_tool_only stays in effect for the turn and the ack gets dropped by dispatch-from-config.ts:2412. Every other text-capable channel already tags typed slash commands correctly: Tlon (extensions/tlon/src/monitor/index.ts:554), Discord (extensions/discord/src/monitor/agent-components.dispatch.ts:233), WhatsApp, and iMessage (added in #82642).

Fix

One line in extensions/mattermost/src/mattermost/monitor.ts:1607, mirroring iMessage PR #82642:

CommandSource: commandAuthorized && isControlCommand ? ("text" as const) : undefined,

Both isControlCommand (line 1321) and commandAuthorized (line 1337) are already in scope. Once the inbound context is tagged correctly, the existing core escape hatch activates for that turn and every slash-command ack on Mattermost text turns becomes visible — not only /new and /reset, but also the ACP success/failure messages at commands-reset.ts:148/153, the soft-reset unavailable notice at commands-reset.ts:52, and the other text-command surfaces — with no per-reply opt-ins.

Why not the approach in #86767?

PR #86767 tried to wrap the /new and /reset reply payloads with markReplyPayloadForSourceSuppressionDelivery in commands-reset.ts. Two problems:

It didn't work. The metadata is stored in a WeakMap keyed on the reply object, but normalizeCommandHandlerResult in commands-core.ts:19-31 spreads the reply into a new object literal (reply: { ...result.reply, replyToId: undefined, replyToCurrent: false }), which is not in the WeakMap. So the dispatch suppression check at dispatch-from-config.ts:2400 never saw the flag and continued to drop the ack. (toEqual tests are blind to WeakMap metadata, which is why CI was green.)
Even if it had worked, it would only have addressed /new and /reset and left the ACP and soft-reset acks at commands-reset.ts:52/148/153 broken — and would have invited per-reply opt-ins for every future slash-command ack.

This PR addresses the ingress layer instead, which is where the other channel extensions already do this work.

Verification

node scripts/run-vitest.mjs extensions/mattermost/src/mattermost/monitor.inbound-system-event.test.ts → 8 passed (8) (includes the new regression test "tags authorized typed text-slash control commands with CommandSource: text").
node scripts/run-vitest.mjs extensions/mattermost/ → 40 files, 436 tests passed.
node scripts/run-vitest.mjs src/auto-reply/command-turn-context.test.ts src/auto-reply/reply/source-reply-delivery-mode.test.ts src/auto-reply/reply/commands-reset-hooks.test.ts → 42 passed (the contract this fix depends on).
node scripts/check-changed.mjs --base origin/main --staged-too → all lanes pass (conflict markers, changelog attributions, wildcard re-export guards, dup-scan coverage, dep-pin, package-patch, media-download helper, runtime-sidecar loader, full typecheck via tsgo, lint shards, runtime import cycles).
oxfmt --check on touched files: clean.

Real behavior proof

Behavior addressed: Typed /new and /reset (including the /<space>command forms used to bypass Mattermost's native slash UI) on Mattermost direct chats or channels running under a message_tool_only source-reply mode (notably the Codex harness default) silently change session state without a visible ✅ acknowledgement.
Real environment tested: Bug source reproduced and fix verified end-to-end via the unit harness on macOS 26 arm64 / Node 25, using the Mattermost monitor inbound test harness. No live Crabbox/Mattermost session was run (this is a one-line ingress tag with the precedent fix already validated in production by iMessage Fix iMessage slash command acknowledgements #82642), but the test asserts the exact ctx-payload field that the downstream suppression check consumes.

Exact steps or command run after this patch:

node scripts/run-vitest.mjs extensions/mattermost/src/mattermost/monitor.inbound-system-event.test.ts -t "text-slash"

Evidence after fix:

 RUN  v4.1.7 /Users/aknight/Development/worktrees/openclaw-mattermost-text-slash
 Test Files  1 passed (1)
      Tests  1 passed | 7 skipped (8)

Negative control — with the extensions/mattermost/src/mattermost/monitor.ts change reverted (test file unchanged), the same run fails at the new assertion:

 ❯ extensions/mattermost/src/mattermost/monitor.inbound-system-event.test.ts:594:32
     592|     expect(ctx?.BodyForAgent).toBe("/reset");
     593|     expect(ctx?.CommandAuthorized).toBe(true);
     594|     expect(ctx?.CommandSource).toBe("text");
        |                                ^
 Tests  1 failed | 7 skipped (8)

i.e. without the fix, the ingress context emits CommandSource: undefined, which is the exact precondition for the suppression cascade in the issue. With the fix, CommandSource: "text" is set, the explicit-command escape hatch activates, and the ack reaches the user.

Observed result after fix: Typed authorized control commands on Mattermost post the appropriate acknowledgement (✅ Session reset., ✅ New session started., ✅ ACP session reset in place., ⚠️ ACP session reset failed. …, Usage: /reset soft …) instead of silently mutating session state.
What was not tested: A live Mattermost direct-chat session under the Codex harness was not exercised on Crabbox/Testbox — the surface change is a single-line ingress tag mirroring the iMessage precedent, and the negative-control unit assertion above exercises the exact contract the downstream suppression check reads.

clawsweeper · 2026-05-26T08:44:39Z

Codex review: needs maintainer review before merge. Reviewed May 26, 2026, 5:18 AM ET / 09:18 UTC.

Summary
This PR tags authorized Mattermost typed control commands with CommandSource: "text" and adds a focused inbound regression test while leaving non-control text untagged.

PR surface: Source +5, Tests +84. Total +89 across 2 files.

Reproducibility: yes. at source level: current main's Mattermost post path sets CommandAuthorized without CommandSource, while the core delivery policy only bypasses message_tool_only for explicit command turns. I did not run tests because this review was read-only.

Review metrics: none identified.

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

Optional before beta landing: attach a redacted live Mattermost smoke showing typed /new or /reset produces a visible acknowledgement under message_tool_only.

Mantis proof suggestion
A short visible Mattermost chat smoke would directly cover the remaining real transport proof gap for acknowledgement delivery. A maintainer can ask Mantis to capture proof by posting a new PR comment that starts with the OpenClaw Mantis account mention, followed by:

visual task: verify in Mattermost with message_tool_only that typed /new and /reset show visible acknowledgements.

Risk before merge

No live Mattermost session under message_tool_only is recorded on the PR; the branch proves the exact inbound context contract with a negative-control unit harness, but not the real transport posting path.

Maintainer options:

Accept the contract proof (recommended)
Land after required checks if maintainers accept the focused inbound-context assertion plus existing source-reply contract tests as sufficient for this one-line Mattermost parity fix.
Add live Mattermost proof
Before landing, run a redacted Mattermost DM or channel smoke under message_tool_only showing typed /new or /reset produces a visible acknowledgement.

Next step before merge
No repair lane is needed: the previous changelog blocker is gone and the remaining action is normal maintainer/CI or live-proof judgment for a protected maintainer PR.

Security
Cleared: The diff only touches Mattermost TypeScript runtime/test code and does not change dependencies, workflows, credentials, package metadata, or other supply-chain surfaces.

Review details

Best possible solution:

Land the ingress-level CommandSource tag with its regression coverage after required checks and maintainer acceptance of the message-delivery proof gap.

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

Yes, at source level: current main's Mattermost post path sets CommandAuthorized without CommandSource, while the core delivery policy only bypasses message_tool_only for explicit command turns. I did not run tests because this review was read-only.

Is this the best way to solve the issue?

Yes, the PR fixes the ingress classification where the missing fact originates, instead of marking individual reset reply payloads and risking future acknowledgement drift.

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 90bcec9fa4a8.

Label changes

Label changes:

add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The external-contributor proof gate is not applied because this is a MEMBER/protected maintainer PR; the PR body provides unit-harness and negative-control evidence but no live Mattermost session.
remove rating: 🦐 gold shrimp: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
remove status: ⏳ waiting on author: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

P1: The PR fixes a beta-blocking Mattermost workflow where state-changing /new or /reset commands can silently change session state without a visible acknowledgement.
merge-risk: 🚨 message-delivery: The patch changes Mattermost source-reply suppression behavior for authorized typed control commands, so the user-visible delivery path matters beyond ordinary unit correctness.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The external-contributor proof gate is not applied because this is a MEMBER/protected maintainer PR; the PR body provides unit-harness and negative-control evidence but no live Mattermost session.

Evidence reviewed

PR surface:

Source +5, Tests +84. Total +89 across 2 files.

View PR surface stats

Area	Files	Added	Net
Source	1	5	+5
Tests	1	84	+84
Docs	0	0	0
Config	0	0	0
Generated	0	0	0
Other	0	0	0
Total	2	89	+89

What I checked:

Current main behavior: The Mattermost regular post path builds the inbound context with CommandAuthorized but no CommandSource, so authorized typed slash commands are still normal message turns on current main. (extensions/mattermost/src/mattermost/monitor.ts:1607, 90bcec9fa4a8)
PR implementation: The PR head adds CommandSource: commandAuthorized && isControlCommand ? ("text" as const) : undefined at the Mattermost ingress point, matching the already-computed control-command and authorization facts. (extensions/mattermost/src/mattermost/monitor.ts:1612, 548c8abdc5d2)
Delivery contract: resolveCommandTurnContext only treats a legacy field as an authorized text-slash turn when CommandSource is text and CommandAuthorized is true; resolveSourceReplyDeliveryMode then returns automatic for explicit command turns before message-tool suppression is applied. (src/auto-reply/command-turn-context.ts:160, 90bcec9fa4a8)
Regression coverage: The PR adds a Mattermost inbound post test for a leading-space /reset DM that asserts BodyForAgent, CommandBody, CommandAuthorized, and CommandSource: "text", plus a negative assertion that inline non-control text remains untagged. (extensions/mattermost/src/mattermost/monitor.inbound-system-event.test.ts:512, 548c8abdc5d2)
Sibling surface precedent: The iMessage inbound path already gates CommandSource: "text" on both command authorization and actual control-command detection, which supports this PR's ingress-level shape over per-reply opt-ins. (extensions/imessage/src/monitor/inbound-processing.ts:945, 90bcec9fa4a8)
Prior review blocker removed: The current PR diff against main touches only the Mattermost monitor and its test; CHANGELOG.md is no longer part of the branch after the follow-up commit. (548c8abdc5d2)

Likely related people:

steipete: Recent GitHub path history shows repeated work on source-reply-delivery-mode.ts, command-turn-context.ts, and Mattermost monitor refactors, including visible-reply and command-turn changes that define the contract this PR uses. (role: core reply-delivery and command-turn contributor; confidence: high; commits: 67b16a4a6d25, 64d4f99d2641, 1e5450f23e1c; files: src/auto-reply/reply/source-reply-delivery-mode.ts, src/auto-reply/command-turn-context.ts, extensions/mattermost/src/mattermost/monitor.ts)
vincentkoc: Recent Mattermost monitor history includes delivery and draft-progress fixes by this contributor near the affected channel runtime surface. (role: recent Mattermost area contributor; confidence: medium; commits: d1cd74b2431d, be438cf887ce, 3546a5400317; files: extensions/mattermost/src/mattermost/monitor.ts)
kinjitakabe: Recent Mattermost delivery work diagnosed silent final-delivery completions in the same channel delivery area, which is adjacent to this PR's acknowledgement-delivery fix. (role: Mattermost delivery diagnostics contributor; confidence: medium; commits: 6789fe248bcf; files: extensions/mattermost/src/mattermost/monitor.ts)
homer-byte: The merged iMessage acknowledgement fix introduced the same authorized-control-command CommandSource: "text" pattern that this Mattermost PR mirrors. (role: sibling-surface precedent contributor; confidence: medium; commits: 4d150209c31b; files: extensions/imessage/src/monitor/inbound-processing.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.

clawsweeper · 2026-05-26T08:50:01Z

ClawSweeper PR egg

✨ Hatched: 🥚 common Velvet Merge Sprite

Hatch command

Comment @clawsweeper hatch when this PR is hatchable.

Hatchability rules:

Merged PRs are hatchable.
Open PRs are hatchable when they are status: 👀 ready for maintainer look, status: 🚀 automerge armed, or labeled clawsweeper:automerge.
Closed unmerged PRs are hatchable only when one of those hatchable labels is still present in the durable record.

Rarity: 🥚 common.
Trait: hums during re-review.
Image traits: location branch lighthouse; accessory tiny test log scroll; palette cobalt, lime, and pearl; mood sparkly; pose standing beside its cracked shell; shell woven fiber shell; lighting calm overcast light; background little resolved-comment flags.
Share on X: post this hatch
Copy: My PR egg hatched a 🥚 common Velvet Merge Sprite 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.
Hatchability usually comes from sufficient real-behavior proof, no blocking P0/P1/P2 findings, no security attention needed, and clean correctness. A merged PR is already final, so merge makes the egg hatchable independently.
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.

…urce so /new and /reset acks survive message_tool_only suppression The Mattermost text-post inbound handler in extensions/mattermost/src/mattermost/monitor.ts already sets CommandAuthorized for typed slash commands, but never set CommandSource. Without CommandSource: 'text', resolveCommandTurnContext defaults the turn kind to 'normal', so the explicit-command exception in source-reply-delivery-mode.ts:54-56 never fires. Under message_tool_only delivery (e.g. the Codex harness default for DMs), the resulting acknowledgements ('✅ Session reset.' / '✅ New session started.', and the ACP/soft-reset variants) were silently dropped — the user-reported beta-blocker in #86664. Adds the CommandSource tag and a focused regression test in monitor.inbound-system-event.test.ts. Mirrors the iMessage fix in #82642. Fixes #86664.

amknight · 2026-05-26T09:18:06Z

Pre-merge verification for PR #86825 at 548c8abdc5d2cc1ea2eb7f2e3aaab979776a0d59.

Local proof after review fix and rebase:

node scripts/run-vitest.mjs extensions/mattermost/src/mattermost/monitor.inbound-system-event.test.ts src/auto-reply/command-turn-context.test.ts src/auto-reply/reply/source-reply-delivery-mode.test.ts src/auto-reply/reply/commands-reset-hooks.test.ts
# 4 files, 50 tests passed

git diff --check origin/main..HEAD
# passed

./node_modules/.bin/oxfmt --check --threads=1 extensions/mattermost/src/mattermost/monitor.inbound-system-event.test.ts extensions/mattermost/src/mattermost/monitor.ts
# passed

AUTOREVIEW_AUTO_TESTS=0 .agents/skills/autoreview/scripts/autoreview --mode local
# clean: no accepted/actionable findings reported

CI on the PR head:

CI run: https://github.com/openclaw/openclaw/actions/runs/26443271650
Real behavior proof: passed.
OpenGrep, Workflow Sanity, CodeQL Critical Quality selected shards, security-fast, build-artifacts, check-prod-types, lint, dependencies, guards, and all selected Node/runtime shards: passed or skipped/neutral where expected.
Known ignored failure: check-test-types failed with extensions/codex/src/app-server/run-attempt.test.ts(6830,49): error TS2345: Argument of type '180000' is not assignable to parameter of type '120000'.

That check-test-types failure is not from this Mattermost PR. It is already present on main at the PR base 0e733795f4fdc51d8151d228ad45d6b26198ab9e in run https://github.com/openclaw/openclaw/actions/runs/26442926352, job https://github.com/openclaw/openclaw/actions/runs/26442926352/job/77841765743, with the same file, line, and TS2345 message.

Known proof gap: no live Mattermost transport run was added; the patch is covered by the Mattermost inbound harness and downstream command-source delivery contract tests.

…ce across all channel ingresses Sweeps the same root-cause fix as PR #86825 (Mattermost) across the remaining channels that exhibited the latent gap: Feishu, IRC, Line, MS Teams, Nextcloud Talk, Signal, ClickClack, QA Channel, and the public 'direct-dm' plugin SDK helper (with Nostr passing through it as the first internal consumer). Each channel inbound handler already computed CommandAuthorized for typed slash commands but never set CommandSource. Without CommandSource: 'text', resolveCommandTurnContext defaults the turn kind to 'normal', so the explicit- command exception in source-reply-delivery-mode.ts:54-56 never fires and acknowledgements ('Session reset.', 'New session started.', 'Stopped N subagents', etc.) are silently dropped under message_tool_only delivery modes — most commonly the Codex harness default for DMs that #83571 introduced. For each channel, this PR sets: CommandSource: commandAuthorized && hasControlCommand ? 'text' : undefined where 'hasControlCommand' uses the channel's existing isControlCommandMessage helper (not shouldComputeCommandAuthorized, which would be too permissive and incorrectly tag inline command tokens within ordinary text). For ClickClack and QA Channel (CommandAuthorized hardcoded true), the tag only depends on the body being a control command. For Signal and Line, a new optional 'hasControlCommand' field on the entry/params struct carries the signal from the access-policy layer where it is already computed. The public direct-dm SDK helper grows an optional commandSource parameter so third-party plugins built on it can pass through their own control-command detection; Nostr is updated as the first consumer. Adds focused regression tests (Signal, MS Teams, direct-dm SDK, plus a negative-control assertion on the existing MS Teams inline-command test) that fail without the fix and pass with it. Fixes #86664.

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

amknight self-assigned this May 26, 2026

amknight added 2 commits May 26, 2026 19:09

fix(mattermost): address text slash review feedback

548c8ab

amknight force-pushed the ak/mattermost-text-slash-command-source branch from 8af4a24 to 548c8ab Compare May 26, 2026 09:11

amknight merged commit 21aebd5 into main May 26, 2026
108 of 111 checks passed

amknight deleted the ak/mattermost-text-slash-command-source branch May 26, 2026 09:18

amknight mentioned this pull request May 26, 2026

fix(channels): tag typed text-slash control commands with CommandSource across all channel ingresses #86863

Closed

amknight mentioned this pull request May 26, 2026

fix(reply): derive source-reply explicit-command bypass from authorized + control-command body #86874

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(mattermost): tag typed text-slash control commands so /new and /reset acks survive message_tool_only suppression#86825

fix(mattermost): tag typed text-slash control commands so /new and /reset acks survive message_tool_only suppression#86825
amknight merged 2 commits into
mainfrom
ak/mattermost-text-slash-command-source

amknight commented May 26, 2026

Uh oh!

clawsweeper Bot commented May 26, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented May 26, 2026 •

edited

Loading

Uh oh!

amknight commented May 26, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

amknight commented May 26, 2026

Summary

Fix

Why not the approach in #86767?

Verification

Real behavior proof

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Hatch command

Uh oh!

amknight commented May 26, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

clawsweeper Bot commented May 26, 2026 •

edited

Loading

clawsweeper Bot commented May 26, 2026 •

edited

Loading