This is a comment left during a code review.
Path: extensions/mattermost/src/channel.ts
Line: 9

Comment:
**Unused import `registerPluginHttpRoute`**

`registerPluginHttpRoute` is imported but never referenced anywhere in this file. This will likely produce a lint/TypeScript warning and should be removed.

```suggestion
  resolveAllowlistProviderRuntimeGroupPolicy,
```

How can I resolve this? If you propose a fix, please make it concise.

This is a comment left during a code review.
Path: extensions/mattermost/src/channel.ts
Line: 25-30

Comment:
**Duplicate import from the same module**

`normalizeMattermostBaseUrl` is imported from `./mattermost/client.js` on its own (line 25), and then `createMattermostClient`, `patchMattermostPost`, and `deleteMattermostPost` are imported in a second block from the same module. These should be merged into a single import statement to avoid the redundancy.

```suggestion
import {
  createMattermostClient,
  deleteMattermostPost,
  normalizeMattermostBaseUrl,
  patchMattermostPost,
} from "./mattermost/client.js";
```

How can I resolve this? If you propose a fix, please make it concise.

This is a comment left during a code review.
Path: extensions/mattermost/src/mattermost/monitor.ts
Line: 938-951

Comment:
**Fallback sends only current block, losing all accumulated text**

When the `patchMattermostPost` call fails for a subsequent block, the code:
1. Appends the current chunk to `blockStreamAccumulatedText` (`+= text`)
2. Clears `blockStreamAccumulatedText = ""`
3. Sends a new message with only `text` (the current block), not the full accumulated text

This means all content from blocks 2 through N-1 (which was stored in `blockStreamAccumulatedText`) is silently discarded. The user ends up with two separate messages: the original post (stuck at the last successfully-patched content) and a new message containing only the current block.

The fallback message should use the accumulated text (captured before clearing), so that at minimum the new message contains everything seen so far:

```typescript
              } catch (err) {
                logVerboseMessage(
                  `mattermost block-stream edit failed: ${String(err)}, falling back to new message`,
                );
                const fullAccumulated = blockStreamAccumulatedText; // already includes += text
                blockStreamMessageId = null;
                blockStreamAccumulatedText = "";
                // Fall through to send as new message
                const result = await sendMessageMattermost(to, fullAccumulated, {
                  accountId: account.accountId,
                  replyToId: threadRootId,
                });
                blockStreamMessageId = result.messageId;
                blockStreamAccumulatedText = fullAccumulated;
              }
```

How can I resolve this? If you propose a fix, please make it concise.

This is a comment left during a code review.
Path: extensions/mattermost/src/mattermost/monitor.ts
Line: 952

Comment:
**Misleading log message after fallback send**

After the catch block completes a fallback by *sending a new message* (not patching an existing one), this log still reads `"block-stream edited …"`. The `blockStreamMessageId` at this point holds the ID of the newly-sent fallback message, so the log is factually wrong and will confuse diagnostics.

```suggestion
              runtime.log?.(`block-stream fallback sent ${blockStreamMessageId}`);
```

How can I resolve this? If you propose a fix, please make it concise.

This is a comment left during a code review.
Path: extensions/mattermost/src/mattermost/monitor.ts
Line: 1271-1279

Comment:
**Orphaned partial message on final-patch failure**

When `patchMattermostPost` fails in the `isFinal` branch, the fallback sends a brand-new message containing the complete final text — but the previously streamed message (whose ID was `blockStreamMessageId`) is left in the channel as an orphaned partial. Users end up seeing two messages: a dangling intermediate post and the new complete one.

Since you already have a reference to the orphaned post via the captured `blockStreamMessageId`, you can attempt to clean it up before (or after) sending the new fallback:

```typescript
              } catch (err) {
                logVerboseMessage(
                  `mattermost block-stream final edit failed: ${String(err)}, sending new message`,
                );
                const orphanId = blockStreamMessageId;
                // Reset state before sending so the new message is tracked correctly.
                blockStreamMessageId = null;
                blockStreamAccumulatedText = "";
                // Best-effort delete of the orphaned partial stream message.
                try {
                  await deleteMattermostPost(blockStreamingClient, orphanId);
                } catch {
                  // Ignore — the delete is a courtesy cleanup.
                }
                await sendMessageMattermost(to, finalText, {
                  accountId: account.accountId,
                  replyToId: threadRootId,
                });
                return;
              }
```

Note that `deleteMattermostPost` is already imported and available in `monitor.ts` (via `client.ts`); it just needs to be added to the import list from that module.

How can I resolve this? If you propose a fix, please make it concise.

This is a comment left during a code review.
Path: extensions/mattermost/src/mattermost/monitor.ts
Line: 1332

Comment:
**`deleteMattermostPost` used but never imported**

`deleteMattermostPost` is called here but is absent from the `./client.js` import block at the top of the file (only `patchMattermostPost` was added). This will fail TypeScript compilation and throw a `ReferenceError` at runtime whenever the `patchMattermostPost` catch-branch is hit.

The import on line 34 needs to include `deleteMattermostPost`:

```suggestion
                await deleteMattermostPost(blockStreamingClient!, orphanId);
```

And the import block in `monitor.ts` should be updated to:
```typescript
import {
  createMattermostClient,
  deleteMattermostPost,
  fetchMattermostChannel,
  fetchMattermostMe,
  fetchMattermostUser,
  fetchMattermostUserTeams,
  normalizeMattermostBaseUrl,
  patchMattermostPost,
  sendMattermostTyping,
  type MattermostChannel,
  type MattermostPost,
  type MattermostUser,
} from "./client.js";
```

How can I resolve this? If you propose a fix, please make it concise.

This is a comment left during a code review.
Path: extensions/mattermost/src/mattermost/monitor.ts
Line: 1238-1244

Comment:
**Missing error handling for first-message send in `flushPendingPatch`**

When `streamMessageId` is `null` (the first partial-reply message has not yet been sent), `sendMessageMattermost` is awaited without a `try/catch`. If this call throws, the error propagates out of `flushPendingPatch` and then out of the `deliver` function, which could prevent the final reply from reaching the user. The `else` branch directly below wraps its patch call in a `try/catch` — this branch should do the same to stay consistent and resilient:

```suggestion
        try {
          const result = await sendMessageMattermost(to, text, {
            accountId: account.accountId,
            replyToId: threadRootId,
          });
          streamMessageId = result.messageId;
          runtime.log?.(`stream-patch started ${streamMessageId}`);
        } catch (err) {
          logVerboseMessage(`mattermost stream-patch flush send failed: ${String(err)}`);
        }
```

How can I resolve this? If you propose a fix, please make it concise.

This is a comment left during a code review.
Path: extensions/mattermost/src/mattermost/monitor.ts
Line: 1280-1311

Comment:
**In-flight interval patch can overwrite the final authoritative message**

The `setInterval` callback uses a fire-and-forget (`void (async () => {...})()`) pattern. The `lastSentText` guard is set synchronously before the network call starts:

```typescript
lastSentText = text;   // set synchronously
patchSending = true;
void (async () => {
  // in-flight patchMattermostPost — still running
})();
```

When `deliver` is called with `isFinal` immediately after a tick fires, `flushPendingPatch` calls `stopPatchInterval()` but then returns early because `text === lastSentText`. The final `patchMattermostPost` is then issued concurrently with the still-in-flight tick's patch. If the Mattermost server processes the partial-text patch *after* the final-text patch, the post ends up permanently showing the truncated partial content instead of the complete answer.

To fix this reliably, replace the `patchSending: boolean` flag with a tracked promise so `flushPendingPatch` can await the in-flight call before issuing the authoritative final patch:

```typescript
// Replace:
let patchSending = false;

// With:
let patchInFlight: Promise<void> | null = null;
```

Then in the interval tick, assign and await the promise; and in `flushPendingPatch`, `await patchInFlight` before proceeding. This ensures the final patch always lands last.

How can I resolve this? If you propose a fix, please make it concise.

This is a comment left during a code review.
Path: extensions/mattermost/openclaw.plugin.json
Line: 3-5

Comment:
**Schema now accepts any configuration key without validation**

Switching from `"additionalProperties": false, "properties": {}` to `"additionalProperties": true` removes all schema enforcement for the plugin config. Typos in config keys (e.g. `blockstreaming` instead of `blockStreaming`) will silently pass validation and be ignored at runtime instead of producing a useful error.

Consider explicitly enumerating the known properties (including the new `blockStreaming` boolean) rather than allowing everything:

```json
{
  "type": "object",
  "additionalProperties": false,
  "properties": {
    "blockStreaming": { "type": "boolean" }
  }
}
```

This keeps validation strict while accommodating the new option.

How can I resolve this? If you propose a fix, please make it concise.

This is a comment left during a code review.
Path: extensions/mattermost/src/mattermost/monitor.ts
Line: 1752-1754

Comment:
**Unbounded spin-wait can hang indefinitely**

The `while (patchSending)` loop has no timeout or maximum iteration count. `patchSending` is only reset to `false` inside the `finally` block of the fire-and-forget async in `schedulePatch`. If the in-flight `updateMattermostPost` or `sendMessageMattermost` network call never resolves (e.g., Mattermost is unreachable or the request stalls), this loop will spin in 20 ms increments forever — permanently blocking the `deliver` function, which prevents the final reply from ever reaching the user.

Consider adding a reasonable timeout (e.g. 5–10 seconds) before breaking out:

```typescript
          const spinDeadline = Date.now() + 10_000; // 10 s max
          while (patchSending && Date.now() < spinDeadline) {
            await new Promise<void>((r) => setTimeout(r, 20));
          }
```

How can I resolve this? If you propose a fix, please make it concise.

This is a comment left during a code review.
Path: extensions/mattermost/src/mattermost/monitor.ts
Line: 1759-1783

Comment:
**Media URLs silently dropped when streaming is active**

`mediaUrls` is populated on line 1743, but both exit paths inside the `if (isFinal && streamMessageId && text)` block — the success path (line 1782 `return`) and the fallback path (line 1779 `return`) — skip the media delivery loop entirely. If the final `ReplyPayload` carries media attachments (e.g. a generated image with a caption), they are silently discarded and never sent to the channel.

The fallback path already calls `sendMessageMattermost` for the text; it should also iterate over any `mediaUrls` afterward. The success path should similarly send any accompanying media after the final patch succeeds:

```typescript
          if (isFinal && streamMessageId && text) {
            try {
              await updateMattermostPost(blockStreamingClient!, streamMessageId, { message: text });
              runtime.log?.(`stream-patch final edit ${streamMessageId}`);
            } catch (err) {
              // ... orphan cleanup + fallback text send ...
              // then fall through to send media below
            }
            streamMessageId = null;
            // fall through to send any media attachments
          }

          // Then send media (runs whether streaming text was used or not):
          for (const mediaUrl of mediaUrls) { ... }
```

How can I resolve this? If you propose a fix, please make it concise.