fix(mattermost): add timeout to REST client requests by Alix-007 · Pull Request #102027 · openclaw/openclaw

Alix-007 · 2026-07-08T04:44:18Z

What Problem This Solves

Fixes an issue where Mattermost message delivery and lookup workflows could hang when the Mattermost REST API accepts a request but never sends a response.

Why This Change Was Made

The shared Mattermost REST client now applies a 30s default request timeout while preserving SSRF/private-network policy and caller abort signals. Per-request timeoutMs remains supported so existing Mattermost DM retry attempts can keep their longer retry deadline instead of being preempted by the client default.

User Impact

Users and operators get a bounded Mattermost REST failure instead of a stuck send, reaction, upload, or target-resolution request when a self-hosted or remote server stalls after accepting the connection.

Evidence

TMPDIR=/media/vdc/0668001470/tmp/openclaw-task-tmp node scripts/run-vitest.mjs extensions/mattermost/src/mattermost/client.fetch-timeout.test.ts extensions/mattermost/src/mattermost/client.retry.test.ts extensions/mattermost/src/mattermost/send.test.ts - passed; Test Files 3 passed (3), Tests 59 passed (59).
Maintainer exact-head proof: sanitized AWS Crabbox cbx_720f70198a7e, run run_7a0bad083ef5, passed the timeout, retry, and client suites at fe84027dc79865a3d98291b3b71782cd875a9829 (3 files, 53 tests).
TMPDIR=/media/vdc/0668001470/tmp/openclaw-task-tmp node scripts/run-oxlint.mjs --tsconfig config/tsconfig/oxlint.extensions.json extensions/mattermost/src/mattermost/client.ts extensions/mattermost/src/mattermost/client.fetch-timeout.test.ts extensions/mattermost/src/mattermost/client.retry.test.ts extensions/mattermost/src/mattermost/send.ts extensions/mattermost/src/mattermost/send.test.ts - passed; exit 0.
git diff --check - passed; exit 0.
Mutation check: temporarily removing the guarded request timeoutMs line made the focused Vitest fail with Expected: "rejected" and Received: "pending"; restoring the patch returned the test to green.

Real behavior proof

Behavior addressed: Mattermost REST client requests now have a deadline when the server accepts the request but never responds, without shortening the existing DM retry timeout path.

Real environment tested: local Linux Node source checkout; real node:http loopback server that accepts the connection and never responds, driving the real exported createMattermostClient, fetchMattermostMe, and createMattermostDirectChannelWithRetry paths.

Exact steps or command run after this patch: TMPDIR=/media/vdc/0668001470/tmp/openclaw-task-tmp node scripts/run-vitest.mjs extensions/mattermost/src/mattermost/client.fetch-timeout.test.ts extensions/mattermost/src/mattermost/client.retry.test.ts extensions/mattermost/src/mattermost/send.test.ts

Evidence after fix: the focused timeout test asserts the loopback server receives /api/v4/users/me; without timeout wiring the request stays pending past the race window, and with timeoutMs: 50 it rejects with an abort/timeout-class error. A caller-signal test proves caller aborts are still honored. A DM retry compatibility test configures a 50ms client default and a 250ms direct-channel retry timeout, observes the request still pending at 120ms, then observes timeout-class rejection by 600ms.

Observed result after fix: Test Files 3 passed (3) and Tests 59 passed (59).

What was not tested: a live Mattermost deployment; the hang is reproduced with a real local HTTP server, not the real third-party service.

clawsweeper · 2026-07-08T04:48:48Z

Codex review: needs maintainer review before merge. Reviewed July 9, 2026, 7:07 AM ET / 11:07 UTC.

Summary
The PR adds a 30-second default timeout to the shared Mattermost REST client, supports per-request timeout overrides, adds stalled-loopback regression tests, and updates the shared HTTP test helper to close active sockets.

PR surface: Source +47, Tests +123. Total +170 across 3 files.

Reproducibility: yes. from source and PR proof. Current main's Mattermost client does not pass a timeout into fetchWithSsrFGuard, and the PR demonstrates the accepted-but-never-responding request shape with a real local HTTP server; I did not run tests in this read-only review.

Review metrics: 1 noteworthy metric.

Timeout policy surfaces: 1 default added, 1 per-request override added, 0 config keys added. The default affects every shared Mattermost REST call, while the override preserves the existing DM retry deadline without adding user config.

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

Risk before merge

[P1] Existing Mattermost REST calls that previously waited longer than 30 seconds will now fail at the new default deadline, including very slow self-hosted servers or uploads.
[P2] Because the shared client is used by sends, reactions, uploads, slash command setup, directory lookup, and target resolution, the timeout can change Mattermost delivery outcomes rather than only diagnostics.
[P1] The live PR was mergeable but behind current main at the latest check, so final merge should use a refreshed exact-head review and CI state.

Maintainer options:

Accept bounded Mattermost REST deadlines (recommended)
Land the PR with the 30-second shared deadline after maintainer acceptance that stalled Mattermost requests should fail fast instead of waiting indefinitely.
Add a longer explicit exception
If maintainers know of a legitimate Mattermost REST operation that can exceed 30 seconds, add a targeted per-request timeout override and focused coverage before merge.
Pause for live service proof
Defer merge if maintainers want live Mattermost send or upload evidence before accepting the new default timeout.

Next step before merge

No automated repair is needed because there are no blocking findings; the remaining action is maintainer acceptance of the Mattermost REST timeout compatibility tradeoff and a refreshed exact-head merge check.

Maintainer decision needed

Question: Should Mattermost's shared REST client enforce a 30-second default for all REST requests, including uploads and slow self-hosted deployments, or should a longer per-surface exception be required first?
Rationale: The implementation is narrow and tested, but the default changes upgrade behavior for existing Mattermost users whose requests previously waited indefinitely or longer than 30 seconds.
Likely owner: steipete — He is the live assignee, authored the latest branch test commits, and has substantial recent Mattermost/plugin boundary history.
Options:
- Accept the bounded default (recommended): Merge with the 30-second all-REST deadline as the intended Mattermost safety policy for stalled servers.
- Require a long-request exception: Ask for a targeted documented timeout override for upload or another proven long-running Mattermost path before merge.
- Pause for live Mattermost proof: Hold the PR until maintainers see live Mattermost send or upload evidence for the chosen deadline.

Security
Cleared: The diff adds abort-signal timeout wiring and tests only; it does not add dependencies, workflow execution, secret handling, or broader network permissions.

Review details

Best possible solution:

Land the bounded Mattermost client timeout after maintainers accept the 30-second all-REST default, or add a documented longer per-surface deadline for any proven long-running Mattermost operation before merge.

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

Yes from source and PR proof. Current main's Mattermost client does not pass a timeout into fetchWithSsrFGuard, and the PR demonstrates the accepted-but-never-responding request shape with a real local HTTP server; I did not run tests in this read-only review.

Is this the best way to solve the issue?

Yes, with upgrade-risk acceptance. Using the existing SSRF guard timeout and shared AbortSignal helper inside the Mattermost client is the narrow maintainable fix; a longer per-surface exception is safer only if maintainers expect uploads or self-hosted calls to exceed 30 seconds.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

P2: The PR fixes a bounded-hang problem in one bundled channel plugin with limited blast radius.
merge-risk: 🚨 compatibility: Existing Mattermost requests that previously waited longer than 30 seconds will now fail at the new default deadline.
merge-risk: 🚨 message-delivery: Slow sends, uploads, reactions, or target lookups could fail and prevent Mattermost message delivery where they previously kept waiting.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body gives after-fix real behavior proof using a real local node:http loopback server that accepts a Mattermost REST request and never responds, with focused tests showing timeout rejection and DM retry preservation.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body gives after-fix real behavior proof using a real local node:http loopback server that accepts a Mattermost REST request and never responds, with focused tests showing timeout rejection and DM retry preservation.

Evidence reviewed

PR surface:

Source +47, Tests +123. Total +170 across 3 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	2	54	7	+47
Tests	1	123	0	+123
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	3	177	7	+170

What I checked:

Current main has no Mattermost REST deadline: On current main, createMattermostClient calls fetchWithSsrFGuard without timeoutMs or signal, so a server that accepts a request and never returns headers can leave shared Mattermost REST operations pending. (extensions/mattermost/src/mattermost/client.ts:191, 56096eb8590c)
PR wires timeout at the shared client boundary: The PR resolves the 30-second default, strips the internal timeoutMs field, and passes the resolved timeout into fetchWithSsrFGuard while preserving the existing SSRF/private-network policy. (extensions/mattermost/src/mattermost/client.ts:193, fe84027dc798)
Caller-specific DM retry deadline is preserved: The PR passes the DM retry timeout through createMattermostDirectChannel and covers a shorter client default with a longer DM retry deadline in the new loopback test. (extensions/mattermost/src/mattermost/client.ts:323, fe84027dc798)
Timeout helper and SSRF guard support the chosen mechanism: buildTimeoutAbortSignal creates the timeout/caller-abort signal, and fetchWithSsrFGuard passes that signal into fetch and propagates timeoutMs into dispatcher creation. (src/infra/net/fetch-guard.ts:451, 56096eb8590c)
Real behavior proof is present in the PR discussion: The PR body reports after-fix terminal proof with a real local node:http loopback server that accepts the Mattermost request and never responds, plus focused Mattermost timeout, retry, send, lint, diff-check, and mutation checks. (fe84027dc798)
Related timeout work does not supersede this Mattermost PR: The linked model-timeout issue is closed as implemented in embedded provider paths, and the two Google Meet timeout PRs are merged for separate plugin surfaces; none modifies Mattermost.

Likely related people:

steipete: Peter Steinberger is assigned on the live PR, authored recent Mattermost/client and SDK test-helper refactors, and added the latest test commits on this branch. (role: recent area contributor and assigned follow-up owner; confidence: high; commits: aec24f4599be, 185668f5c54a, 48853f875bb2; files: extensions/mattermost/src/mattermost/client.ts, extensions/mattermost/src/mattermost/send.ts, src/plugin-sdk/test-helpers/http-test-server.ts)
JonathanJing: Authored the merged Mattermost DM retry timeout work that this PR explicitly preserves. (role: feature owner; confidence: high; commits: 2145eb5908c0; files: extensions/mattermost/src/mattermost/client.ts, extensions/mattermost/src/mattermost/send.ts, extensions/mattermost/src/mattermost/client.retry.test.ts)
jacobtomlinson: Authored the extension fetchWithSsrFGuard routing that this PR relies on for SSRF-safe timeout enforcement. (role: adjacent network-boundary contributor; confidence: high; commits: f92c92515bd4; files: extensions/mattermost/src/mattermost/client.ts, src/infra/net/fetch-guard.ts)
Dominic Damoah: Authored the refactor that extracted the Mattermost channel plugin into the extension boundary where this client now lives. (role: introduced extension boundary; confidence: medium; commits: 495a39b5a989; files: extensions/mattermost/src/mattermost/client.ts)
Samuel Alake: Current-main blame for the relevant Mattermost client and test helper lines points at a broad recent merge snapshot, so this is useful only as weak provenance. (role: recent snapshot contributor; confidence: low; commits: 3f2466c4c378; files: extensions/mattermost/src/mattermost/client.ts, src/plugin-sdk/test-helpers/http-test-server.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.

Review history (8 earlier review cycles)

reviewed 2026-07-08T04:59:20.699Z sha 61c8709 :: needs changes before merge. :: [P1] Preserve configured DM retry timeouts
reviewed 2026-07-08T05:32:05.503Z sha 66331d4 :: needs maintainer review before merge. :: none
reviewed 2026-07-08T05:39:26.932Z sha 66331d4 :: needs maintainer review before merge. :: none
reviewed 2026-07-08T10:15:08.635Z sha 6ad6057 :: needs maintainer review before merge. :: none
reviewed 2026-07-08T10:23:41.846Z sha 6ad6057 :: needs maintainer review before merge. :: none
reviewed 2026-07-09T06:39:24.607Z sha 95cb372 :: needs maintainer review before merge. :: none
reviewed 2026-07-09T08:45:37.225Z sha 84f04ac :: needs maintainer review before merge. :: none
reviewed 2026-07-09T09:21:59.915Z sha 5c224c4 :: needs maintainer review before merge. :: none

Alix-007 · 2026-07-08T10:09:17Z

Merged current upstream/main to refresh the stale failed CI run. The Mattermost change remains limited to the REST client timeout path, including the existing DM retry preservation.

Local validation on the new head:

node scripts/run-vitest.mjs extensions/mattermost/src/mattermost/client.fetch-timeout.test.ts extensions/mattermost/src/mattermost/client.retry.test.ts (2 files, 26 tests passed)
node_modules/.bin/oxlint extensions/mattermost/src/mattermost/client.ts extensions/mattermost/src/mattermost/client.fetch-timeout.test.ts extensions/mattermost/src/mattermost/client.retry.test.ts
node scripts/run-tsgo.mjs -p tsconfig.core.json --incremental false

@clawsweeper re-review

clawsweeper · 2026-07-08T10:09:20Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

steipete · 2026-07-09T11:12:14Z

Maintainer review/proof complete.

Best-fix verdict: yes. createMattermostClient is the shared owner for all Mattermost REST operations, while the per-request override preserves the intentionally longer DM-creation retry deadline and caller aborts remain composed.
Refactored the bespoke 175-line socket harness onto the shared withServer helper. During review, the timeout mutation exposed that shared helper shutdown could wait forever on an active hung socket; the helper now closes active connections before awaiting server shutdown.
Exact head fe84027dc79865a3d98291b3b71782cd875a9829: sanitized AWS Crabbox lease cbx_720f70198a7e, run run_7a0bad083ef5, passed the timeout, retry, and client suites (53 tests). Final autoreview clean; exact-head hosted CI green.
Removed the unrelated Related: #95865 body reference. That closed issue concerns model-generation timeouts, not Mattermost transport requests.

Known proof gap: no live Mattermost deployment was available. The loopback transport proof covers default timeout, caller abort, and longer per-request retry override behavior.

steipete · 2026-07-09T11:12:34Z

Merged via squash.

Prepared head SHA: fe84027dc79865a3d98291b3b71782cd875a9829
Landed commit: 65b2e7a4e337e7f1ea483ff0b617ccda9b95cb13

Prevent Tlon REST client requests from hanging when the server accepts a request but never responds. Add a default 30s timeout to all fetchWithSsrFGuard calls in tlon-api.ts, following the same pattern as PR openclaw#102027 (Mattermost timeout fix). Changes: - Add TLON_API_REQUEST_TIMEOUT_MS constant (30 seconds) - Pass timeoutMs to getMemexUploadUrl fetchWithSsrFGuard call - Pass timeoutMs to Memex upload fetchWithSsrFGuard call - Pass timeoutMs to custom S3 upload fetchWithSsrFGuard call This ensures bounded failures instead of stuck upload requests when self-hosted or remote servers stall after accepting connections.

* fix(mattermost): add timeout to REST client requests * fixup: preserve Mattermost DM retry timeout * test(mattermost): reuse hanging server helper * test(mattermost): satisfy timeout lint --------- Co-authored-by: Peter Steinberger <[email protected]>

openclaw-barnacle Bot added size: M and removed size: S labels Jul 8, 2026

This was referenced Jul 8, 2026

fix(google-meet): bound Meet v2 + Drive files.export request deadlines #102149

Merged

fix(google-meet): bound Calendar v3 events.list request deadline #102157

Merged

clawsweeper Bot added the merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. label Jul 9, 2026

steipete self-assigned this Jul 9, 2026

steipete force-pushed the alix/mattermost-client-fetch-timeout branch from 5c224c4 to fb13567 Compare July 9, 2026 10:52

openclaw-barnacle Bot added size: S and removed size: M labels Jul 9, 2026

Alix-007 and others added 4 commits July 9, 2026 07:00

fix(mattermost): add timeout to REST client requests

545fa44

fixup: preserve Mattermost DM retry timeout

c720f5b

test(mattermost): reuse hanging server helper

c8a5026

test(mattermost): satisfy timeout lint

fe84027

steipete force-pushed the alix/mattermost-client-fetch-timeout branch from fb13567 to fe84027 Compare July 9, 2026 11:00

steipete merged commit 65b2e7a into openclaw:main Jul 9, 2026
104 checks passed

wendy-chsy mentioned this pull request Jul 9, 2026

fix(msteams): add configurable timeout to Graph REST requests #102790

Closed

4 tasks

maweibin mentioned this pull request Jul 9, 2026

fix(googlechat): add 30s request timeout to shared Chat API wrapper #102870

Closed

chengzhichao-xydt mentioned this pull request Jul 9, 2026

fix(voice-call): add timeout to guardedJsonApiRequest #102884

Merged

maweibin mentioned this pull request Jul 9, 2026

fix(qqbot): add 30s request timeout to token endpoint fetch #102897

Merged

chengzhichao-xydt mentioned this pull request Jul 9, 2026

fix(tlon): add timeouts to file upload HTTP requests #102903

Merged

Simon-XYDT mentioned this pull request Jul 9, 2026

fix(tlon): add timeout to REST API requests #102985

Closed

Monkey-wusky mentioned this pull request Jul 10, 2026

fix(googlechat): add timeout to Google auth fetch requests #103627

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(mattermost): add timeout to REST client requests#102027

fix(mattermost): add timeout to REST client requests#102027
steipete merged 4 commits into
openclaw:mainfrom
Alix-007:alix/mattermost-client-fetch-timeout

Alix-007 commented Jul 8, 2026 •

edited by steipete

Loading

Uh oh!

clawsweeper Bot commented Jul 8, 2026 •

edited

Loading

Uh oh!

Alix-007 commented Jul 8, 2026

Uh oh!

clawsweeper Bot commented Jul 8, 2026 •

edited

Loading

Uh oh!

steipete commented Jul 9, 2026

Uh oh!

Uh oh!

steipete commented Jul 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

Alix-007 commented Jul 8, 2026 • edited by steipete 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

Real behavior proof

Uh oh!

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

Uh oh!

Uh oh!

Alix-007 commented Jul 8, 2026

Uh oh!

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

Uh oh!

Uh oh!

steipete commented Jul 9, 2026

Uh oh!

Uh oh!

steipete commented Jul 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Alix-007 commented Jul 8, 2026 •

edited by steipete

Loading

clawsweeper Bot commented Jul 8, 2026 •

edited

Loading

clawsweeper Bot commented Jul 8, 2026 •

edited

Loading