refactor(mattermost): trim monitor gating facade by steipete · Pull Request #106021 · openclaw/openclaw

steipete · 2026-07-13T04:35:49Z

What Problem This Solves

The shrink-only unused-export ratchet still carried seven Mattermost monitor symbols that had no production consumers. Five were re-exported through the monitor facade only for tests, while two input types were public only to type test helpers.

Why This Change Was Made

Remove the unused facade and type exports. Tests now import the owning gating module directly and derive private input types from the exported function signature, leaving runtime imports and behavior unchanged.

User Impact

No user-visible behavior change. This narrows the internal Mattermost API and reduces the unused-export baseline from 4,986 to 4,979 entries.

Evidence

pnpm deadcode:exports:update on Linux: 4,979 entries
pnpm deadcode:exports: matched 4,979 entries
Baseline SHA-256: a6b1ad803cb9185a4e1fc66f4153d64d5fe6667e2b0a9c978e690c9990a74927
Mattermost focused tests: 72 passed across 4 files
oxfmt --check on all touched files: passed
pnpm check:loc: passed
pnpm tsgo:extensions: passed
pnpm tsgo:extensions:test: passed
Mattermost monitor LOC ratchet: 2,476 to 2,466 lines
Fresh branch autoreview: clean, 0.98 confidence

AI assistance: yes. Agent transcript not included pending explicit opt-in.

steipete · 2026-07-13T05:04:05Z

Land-ready local proof for 91103acc299e068157b8f08921a64164852d258e:

Linux pnpm deadcode:exports:update + pnpm deadcode:exports: 4,979 entries, matched; SHA-256 a6b1ad803cb9185a4e1fc66f4153d64d5fe6667e2b0a9c978e690c9990a74927
pnpm check:loc: pass
pnpm tsgo:extensions + pnpm tsgo:extensions:test: pass
focused Mattermost tests: 72/72 pass across four files
touched-file formatting: pass
monitor LOC ratchet: 2,476 to 2,466 lines
fresh branch autoreview: clean, 0.98 confidence
Testbox leases tbx_01kxcz3hm6f7v7h3pyyksck3bb and tbx_01kxd02eq97v0hh99r7gwdh7m8
current-main merge-tree proof against a0e5c159888c1287c2606d185b28b43446afd1d0: conflict-free; merged deadcode baseline blob equals the reviewed branch blob

Exact-head CI run https://github.com/openclaw/openclaw/actions/runs/29227391969: rollup SUCCESS; 77 successful, 38 skipped, 1 neutral, zero failures after rerunning the installation-quota-limited detector and CodeQL jobs.

clawsweeper · 2026-07-13T05:18:20Z

Codex review: needs real behavior proof before merge. Reviewed July 13, 2026, 1:59 AM ET / 05:59 UTC.

Summary
Removes seven unused Mattermost monitor facade and helper-type exports, redirects tests to the owning gating module, and lowers the dead-code export and monitor LOC baselines.

PR surface: Source -10, Tests +3, Other -8. Total -15 across 6 files.

Reproducibility: not applicable. This PR removes unused internal exports rather than addressing a runtime failure, and the relevant change is directly auditable from the import and export graph.

Review metrics: 1 noteworthy metric.

Internal export surface: 7 removed, 0 added. The PR narrows unused internal and test-facing symbols without introducing a parallel API or compatibility path.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🦞 diamond lobster
Result: blocked until real behavior proof from a real setup is added.

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

Rank-up moves:

Provide a redacted Mattermost runtime transcript or logs for the exact head, or obtain an explicit maintainer proof override.

Proof guidance:

[P1] Needs real behavior proof before merge: Available evidence is limited to tests, type checks, CI/Testbox output, static ratchets, and merge-tree proof; add redacted live Mattermost logs or a transcript, or obtain a maintainer proof override, then update the PR body to retrigger review or request @clawsweeper re-review.

Risk before merge

[P1] The external-contributor proof requirement remains unresolved: tests, type checks, CI, Testbox output, and static ratchets do not demonstrate the behavior in a real Mattermost setup.
[P1] The protected maintainer label requires explicit human handling even though the patch itself appears correct.

Maintainer options:

Decide the mitigation before merge
Merge the bounded cleanup after explicit maintainer handling and either a narrow redacted Mattermost runtime smoke or a proof override recognizing that the patch changes export visibility rather than runtime logic.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

[P1] No automated repair is needed; a human must resolve the protected-label and proof-override decision before merge.

Maintainer decision needed

Question: Should this export-only Mattermost refactor receive a proof override, or must the contributor add a live Mattermost smoke before merge?
Rationale: The repository requires real behavior proof for external non-docs PRs, but this patch leaves runtime function bodies and production calls unchanged, making a maintainer judgment appropriate on whether live proof is proportionate.
Likely owner: steipete — The available merged history ties this person to both the dead-code ratchet and this Mattermost cleanup surface.
Options:
- Grant proof override (recommended): Accept the focused tests, type checks, ratchet checks, and unchanged runtime path as sufficient evidence for this internal cleanup.
- Require live smoke: Request redacted Mattermost logs or a transcript showing normal message and mention-gating handling on this exact head.

Security
Cleared: The patch adds no dependencies, workflows, permissions, secret handling, artifact execution, or other security or supply-chain surface.

Review details

Best possible solution:

Merge the bounded cleanup after explicit maintainer handling and either a narrow redacted Mattermost runtime smoke or a proof override recognizing that the patch changes export visibility rather than runtime logic.

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

Not applicable: this PR removes unused internal exports rather than addressing a runtime failure, and the relevant change is directly auditable from the import and export graph.

Is this the best way to solve the issue?

Yes. Importing tests from the owning gating module and deriving private parameter types from the function signature is narrower and less drift-prone than retaining a test-only facade.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

P3: This is a low-risk internal cleanup with no intended user-visible behavior change.
rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦞 diamond lobster.
status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: Available evidence is limited to tests, type checks, CI/Testbox output, static ratchets, and merge-tree proof; add redacted live Mattermost logs or a transcript, or obtain a maintainer proof override, then update the PR body to retrigger review or request @clawsweeper re-review.

Evidence reviewed

PR surface:

Source -10, Tests +3, Other -8. Total -15 across 6 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	2	2	12	-10
Tests	2	7	4	+3
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	2	1	9	-8
Total	6	10	25	-15

What I checked:

Changed surface and owner boundary: The gating functions remain implemented and exported by their owning module; the patch only makes two helper input types private and removes the test-only forwarding facade from the monitor module. (extensions/mattermost/src/mattermost/monitor-gating.ts:29, 91103acc299e)
Caller alignment: The main Mattermost monitor continues using the gating implementation internally while no helper body or production call site is changed. (extensions/mattermost/src/mattermost/monitor.ts:129, 91103acc299e)
Test ownership: Focused tests import gating functions from monitor-gating.ts, and test-only parameter types are derived from the function signature rather than maintained as public exports. (extensions/mattermost/src/mattermost/monitor.test.ts:8, 91103acc299e)
Sibling coverage: The channel-kind test is redirected from the monitor facade to the same owning gating module without changing its assertions. (extensions/mattermost/src/mattermost/monitor.channel-kind.test.ts:3, 91103acc299e)
Ratchet consistency: The unused-export baseline removes the same seven symbols eliminated by the TypeScript changes, and the LOC baseline decreases by the ten facade lines removed from monitor.ts. (scripts/deadcode-exports.baseline.mjs:378, 91103acc299e)
Supplemental validation: The PR reports 72 focused Mattermost tests, extension production and test type checks, formatting, dead-code and LOC ratchets, and conflict-free merge-tree proof on the current head; these support correctness but do not constitute observed runtime behavior. (91103acc299e)

Likely related people:

steipete: Prior merged history connects this person to the unused-export ratchet, and the current commits cover the corresponding Mattermost cleanup and baseline maintenance. (role: recent area contributor; confidence: high; commits: a425807c31d3, 22b45652d1a6, 0a1fc6fb91bb; files: extensions/mattermost/src/mattermost/monitor-gating.ts, extensions/mattermost/src/mattermost/monitor.ts, scripts/deadcode-exports.baseline.mjs)

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 (3 earlier review cycles)

reviewed 2026-07-13T05:25:22.061Z sha 43e6fc4 :: needs real behavior proof before merge. :: none
reviewed 2026-07-13T05:37:03.729Z sha 43e6fc4 :: needs real behavior proof before merge. :: none
reviewed 2026-07-13T05:49:52.765Z sha 43e6fc4 :: needs real behavior proof before merge. :: none

* refactor(mattermost): trim monitor gating facade * chore(ci): shrink Mattermost LOC baseline * chore(ci): refresh deadcode export baseline

…hreadDirectMessages The 7.2 cherry-pick resolved extensions/mattermost/src/mattermost/monitor.ts by taking our side wholesale, silently reverting six upstream commits landed between 7.1 and 7.2. Adopt them instead of clobbering: f77c2aa truncate the inbound preview on a code-point boundary (openclaw#101630) b7ab62c preserve text-block boundaries in the draft preview (openclaw#87449) da6a24d prevent hung pre-delivery hooks from blocking lanes (openclaw#104256) 218dcd8 noUncheckedIndexedAccess in the extensions lane (openclaw#105132) 9cdf166 trim the monitor gating facade (openclaw#106021) 70a7e6e (opt-in DM threading by chat type) was already in the impl; three 7.1-era assertions in monitor.test.ts that upstream had superseded move to upstream's semantics (the implementations are byte-identical to upstream). b7ab62c's 173-line monitor hunk (block-preview boundary machinery) was hand-ported onto our activity/warm-fallback interleaving; the block-preview code is now byte-identical to pristine 7.2. threadDirectMessages kept as a thin compat feeding upstream's replyToModeByChatType, because Dockerfile.base's shared config probe still writes it and the probe spans matrix bases older than 2026.7.2. Retire both together later. Mattermost suite 630/632. The 2 remaining timeouts are localized to deliverMattermostReplyWithDraftPreview: with a mocked-rejecting updateMattermostPost, our patched buildFinalEdit attempts the in-place edit (pristine skips it), the edit rejects, and the deliverNormally fallback awaits the unmocked deliverMattermostReplyPayload which never resolves under the test's bare sendMessageMattermost stub. Behavioral divergence in our activity/warm-fallback finalize path, not the upstream port. TODO.

openclaw-barnacle Bot added channel: mattermost Channel integration: mattermost scripts Repository scripts size: XS maintainer Maintainer-authored PR labels Jul 13, 2026

steipete force-pushed the codex/deadcode-exports-mattermost-monitor branch from 8475203 to 9de3674 Compare July 13, 2026 04:50

steipete force-pushed the codex/deadcode-exports-mattermost-monitor branch from 9de3674 to 43e6fc4 Compare July 13, 2026 05:16

steipete added 3 commits July 13, 2026 06:43

refactor(mattermost): trim monitor gating facade

22b4565

chore(ci): shrink Mattermost LOC baseline

0a1fc6f

chore(ci): refresh deadcode export baseline

91103ac

steipete force-pushed the codex/deadcode-exports-mattermost-monitor branch from 43e6fc4 to 91103ac Compare July 13, 2026 05:52

steipete merged commit 9cdf166 into main Jul 13, 2026
136 of 148 checks passed

steipete deleted the codex/deadcode-exports-mattermost-monitor branch July 13, 2026 06:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

refactor(mattermost): trim monitor gating facade#106021

refactor(mattermost): trim monitor gating facade#106021
steipete merged 3 commits into
mainfrom
codex/deadcode-exports-mattermost-monitor

steipete commented Jul 13, 2026 •

edited

Loading

Uh oh!

steipete commented Jul 13, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jul 13, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

steipete commented Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Uh oh!

steipete commented Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

steipete commented Jul 13, 2026 •

edited

Loading

steipete commented Jul 13, 2026 •

edited

Loading

clawsweeper Bot commented Jul 13, 2026 •

edited

Loading