Skip to content

fix(mattermost): anchor slash state on globalThis (#68113)#90389

Closed
ly85206559 wants to merge 1 commit into
openclaw:mainfrom
ly85206559:fix/mattermost-slash-68113
Closed

fix(mattermost): anchor slash state on globalThis (#68113)#90389
ly85206559 wants to merge 1 commit into
openclaw:mainfrom
ly85206559:fix/mattermost-slash-68113

Conversation

@ly85206559

@ly85206559 ly85206559 commented Jun 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

  • Fixes Mattermost native slash commands returning permanent HTTP 503 with "Slash commands are not yet initialized" even after logs show slash commands activated (regression reported in [Bug]: Mattermost slash commands return 503 "not yet initialized" in v2026.4.15 #68113 on v2026.4.15+).
  • Root cause: slash callback HTTP route registration (jiti-loaded plugin bundle) and monitor activation (native ESM) each held a separate module-level accountStates Map, so the route handler never saw tokens/handlers populated by activateSlashCommands().
  • Fix: anchor accountStates on a process-wide globalThis singleton via Symbol.for("openclaw.mattermost.slash-account-states"), matching the approach from closed PR fix(mattermost): anchor slash command state to globalThis to fix dual-instance 503s #69038.
  • Adds regression coverage that the shared map survives a simulated second module load (vi.resetModules()), modeling the dual-loader failure mode.

Intentionally out of scope: Mattermost API registration changes, multi-account routing policy, nginx/proxy config, or unrelated channel work.

Reviewer focus: Confirm shutdown/deactivate still clears global state; multi-account ambiguous-token behavior unchanged; no new cross-plugin globals beyond the documented symbol.

Linked context

Closes #68113

Related #69038

Real behavior proof

  • Behavior or issue addressed: Mattermost slash HTTP callback stayed on 503 "not yet initialized" forever while startup logs claimed slash commands were active; users could not run /oc_* commands ([Bug]: Mattermost slash commands return 503 "not yet initialized" in v2026.4.15 #68113).
  • Real environment tested: Windows 11 dev host, OpenClaw built from this PR branch (fix/mattermost-slash-68113), local gateway on loopback with Mattermost channel configured for native slash commands (same callback path as issue reporter).
  • Exact steps or command run after this patch:
    1. git checkout fix/mattermost-slash-68113 and build/install the local tree.
    2. Ensure channels.mattermost.commands.native (and callback URL/path) are enabled in config, matching the issue setup.
    3. Start gateway: pnpm openclaw gateway run --bind loopback --port 18789 --force
    4. Wait until gateway log contains mattermost: slash commands activated for the account.
    5. Probe the registered callback route (no Mattermost POST body required for this check): curl -sk -o NUL -w "HTTP %{http_code}\n" http://127.0.0.1:18789/api/channels/mattermost/command
    6. (Optional) Execute /oc_status in Mattermost against the configured callback URL to confirm end-user slash delivery when a server is available.
  • Evidence after fix (terminal capture — redacted hostnames): 
    [gateway] mattermost: slash commands activated for account default (33 commands)
[gateway] ready (...)

PS> curl -sk -o NUL -w "HTTP %{http_code}\n" http://127.0.0.1:18789/api/channels/mattermost/command
HTTP 405
    Before this patch on the same setup, the same GET returned HTTP 503 with JSON "Slash commands are not yet initialized..." (see [Bug]: Mattermost slash commands return 503 "not yet initialized" in v2026.4.15 #68113 logs).
  • Observed result after fix: After activation, the callback route is wired to initialized slash state (GET returns 405 Method Not Allowed, not 503 uninitialized). This matches the issue's expected "handler is up" signal; Mattermost POST handling uses the same shared accountStates map.
  • What was not tested: Full Mattermost-hosted slash roundtrip on a shared production gateway behind nginx in this verification pass (no dedicated MM server in the capture above). Issue reporter environment remains the bar for full channel acceptance.
  • Proof limitations or environment constraints: Evidence is from a local loopback gateway + curl probe plus gateway runtime logs. Maintainer with Mattermost can confirm POST slash execution on the same build.

Tests and validation

  • pnpm test extensions/mattermost/src/mattermost/slash-state.test.ts — 6 passed (includes globalThis singleton + module-reload regression)
  • CI on PR head: all required checks green except this proof gate until body update lands

Risk checklist

Did user-visible behavior change? Yes — Mattermost slash callbacks stop returning permanent 503 after activation.

Could this break multi-account slash routing? Low — only shares state that was always intended to be shared; ambiguous token/conflict paths unchanged.

Could global state leak across restarts? Mitigated — deactivateSlashCommands() still clears entries; symbol is process-scoped.

What reviewers should focus on

@openclaw-barnacle openclaw-barnacle Bot added channel: mattermost Channel integration: mattermost size: S triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI. labels Jun 4, 2026
@clawsweeper

clawsweeper Bot commented Jun 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 4, 2026, 12:23 PM ET / 16:23 UTC.

Summary
This PR moves Mattermost slash-command account state from a module-local Map to a Symbol.for globalThis singleton and adds regression coverage for state surviving a second module load.

PR surface: Source +21, Tests +43. Total +64 across 2 files.

Reproducibility: yes. The linked issue provides concrete Mattermost setup steps and current source shows the callback route returns 503 when its module-local state is empty while activation happens through a separate monitor path; I did not run a live Mattermost server in this read-only review.

Review metrics: none identified.

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

Mantis proof suggestion
A real Mattermost slash command in the UI would close the only remaining proof gap beyond the local callback probe. 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 a Mattermost `/oc_status` slash command returns a response after gateway logs `mattermost: slash commands activated` on this PR head.

Risk before merge

  • [P1] Contributor proof covers gateway activation plus a callback route probe, but not a full hosted Mattermost POST roundtrip behind a reverse proxy.

Maintainer options:

  1. Decide the mitigation before merge
    Land the focused Mattermost plugin state-sharing fix after normal maintainer and CI review; a live Mattermost slash POST smoke would be useful acceptance proof but does not appear to require a different code shape.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • No ClawSweeper repair lane is needed; the patch has no blocking findings and needs ordinary maintainer review plus optional live Mattermost acceptance.

Security
Cleared: No dependency, workflow, package, or external code-execution surface changes were found; the new state is process-local and deactivation still clears it.

Review details

Best possible solution:

Land the focused Mattermost plugin state-sharing fix after normal maintainer and CI review; a live Mattermost slash POST smoke would be useful acceptance proof but does not appear to require a different code shape.

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

Yes. The linked issue provides concrete Mattermost setup steps and current source shows the callback route returns 503 when its module-local state is empty while activation happens through a separate monitor path; I did not run a live Mattermost server in this read-only review.

Is this the best way to solve the issue?

Yes. Anchoring the one intended per-process Mattermost slash state on a Symbol.for globalThis key is a narrow fix that matches existing OpenClaw process-local singleton patterns and avoids a larger plugin-loader refactor.

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 473f651e0933.

Label changes

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix terminal evidence from a Windows local gateway showing activation logs and a callback GET returning HTTP 405 instead of the prior 503 uninitialized response.
  • add rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • remove rating: 🐚 platinum hermit: Current PR rating is rating: 🦞 diamond lobster, so this older rating label is no longer current.

Label justifications:

  • P1: The linked regression blocks Mattermost native slash-command workflows for affected users, while the PR is a narrow channel fix.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes after-fix terminal evidence from a Windows local gateway showing activation logs and a callback GET returning HTTP 405 instead of the prior 503 uninitialized response.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix terminal evidence from a Windows local gateway showing activation logs and a callback GET returning HTTP 405 instead of the prior 503 uninitialized response.
Evidence reviewed

PR surface:

Source +21, Tests +43. Total +64 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 23 2 +21
Tests 1 44 1 +43
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 67 3 +64

What I checked:

Likely related people:

  • Peter Steinberger: Current blame for the Mattermost slash-state and route boundary lines points to e2a5823, and related bundled-channel boundary refactors include 3126809 and 0c278bb. (role: recent area contributor and boundary refactor author; confidence: high; commits: e2a5823c83c8, 3126809cb06a, 0c278bb93c2d; files: extensions/mattermost/src/mattermost/slash-state.ts, extensions/mattermost/index.ts, extensions/mattermost/slash-route-api.ts)
  • Muhammed Mukhthar CM: The native Mattermost slash-command support commit introduced slash-state.ts, slash-state.test.ts, and the Mattermost entrypoint changes central to this PR. (role: feature introducer; confidence: high; commits: b1b41eb44323; files: extensions/mattermost/src/mattermost/slash-state.ts, extensions/mattermost/src/mattermost/slash-state.test.ts, extensions/mattermost/index.ts)
  • Ayaan Zaidi: Commit 17573d0 routed the Mattermost bundled entry through the plugin API, which is adjacent to the route-registration boundary involved here. (role: adjacent boundary contributor; confidence: medium; commits: 17573d097b2f; files: extensions/mattermost/index.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 clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P1 High-priority user-facing bug, regression, or broken workflow. labels Jun 4, 2026
@openclaw-barnacle openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI. labels Jun 4, 2026
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 4, 2026
@ly85206559 ly85206559 force-pushed the fix/mattermost-slash-68113 branch from 96bf3d3 to d1d2aec Compare June 4, 2026 16:16
@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 4, 2026
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jun 4, 2026
@Takhoffman

Takhoffman commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

@clawsweeper automerge

@clawsweeper

clawsweeper Bot commented Jun 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🔧
ClawSweeper automerge is enabled.

Draft PRs stay fix-only until GitHub marks them ready for review. Pause with /clawsweeper stop.

Automerge progress:

  • 2026-06-05 03:16:48 UTC review queued d1d2aece69de (queued)

@clawsweeper clawsweeper Bot added the clawsweeper:automerge Maintainer opted this PR into bounded ClawSweeper-reviewed automerge label Jun 5, 2026
@github-actions github-actions Bot mentioned this pull request Jun 5, 2026
Open
@clawsweeper clawsweeper Bot mentioned this pull request Jun 5, 2026
Merged
@clawsweeper

clawsweeper Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper 🐠 reef update

Thanks for the useful work here. ClawSweeper could not update this branch directly, so the replacement PR is the writable swim lane for the same fix path.

Why replacement: ClawSweeper could not update the source PR branch directly; GitHub did not grant sufficient push rights to the bot for that branch.
Replacement PR: #90534
Why close: this run explicitly closes the superseded source PR after the credited replacement PR is open, so review continues in one place.
Closing this one because the run was configured to close superseded source PRs after opening the replacement.
Attribution stays attached; the replacement just gives the fix a writable branch.
Co-author credit kept:

fish notes: model gpt-5.5, reasoning high; reviewed against 3cf28a1.

@clawsweeper clawsweeper Bot closed this Jun 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

channel: mattermost Channel integration: mattermost clawsweeper:automerge Maintainer opted this PR into bounded ClawSweeper-reviewed automerge P1 High-priority user-facing bug, regression, or broken workflow. proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: Mattermost slash commands return 503 "not yet initialized" in v2026.4.15

2 participants

@ly85206559 @Takhoffman