Promote existing threaded replyToMode=first sessions to all

When the inbound Mattermost message is already inside a thread, extensions/mattermost/src/mattermost/monitor.ts always keeps normal replies on that thread by returning the existing threadRootId regardless of replyToMode. This new buildToolContext path preserves "first" instead, so once hasRepliedRef becomes true, later message send tool calls stop inheriting the active thread and post back to the channel root. That regresses any threaded channel/group conversation configured with replyToMode: first whenever the agent emits a follow-up tool send after its first reply.

Useful? React with 👍 / 👎.

Addressed in commit 3c183d1: for replyToMode=first, hasRepliedRef is now explicitly flipped to true after the first implicit thread send. Subsequent sends in the same turn see hasRepliedRef.value===true and fall through to channel root, matching how monitor.ts handles inbound replies (it keeps them on the thread for the reply path, but tool sends are separately gated by the ref).

Consume hasRepliedRef when the first tool send inherits a thread

For replyToMode="first", this branch only reads toolContext.hasRepliedRef and never flips it after auto-injecting sessionThreadTs. As a result, if the agent issues multiple Mattermost message send actions in one turn before any non-tool reply path updates the ref, every send will be threaded instead of just the first one. The Slack action runtime avoids this by marking the ref as soon as it consumes the first threaded send.

Useful? React with 👍 / 👎.

Fixed in commit 3c183d1: handleAction now sets toolContext.hasRepliedRef.value = true after using sessionThreadTs as the implicit replyToId when replyToMode=first. Test updated to assert hasRepliedRef.value is true after the first implicit send.

replyToMode=first: hasRepliedRef.value is never set to true

When replyToMode === "first" and hasRepliedRef.value === false, the code correctly inherits the thread context — but it never flips hasRepliedRef.value to true after doing so. This means every subsequent call through this path also sees hasRepliedRef.value === false and keeps inheriting the thread, making first behave identically to all for all tool-send calls.

The Slack equivalent (in extensions/slack/src/action-runtime.ts:113-116) performs the mutation before returning the thread ID:

if (context.replyToMode === "first" && context.hasRepliedRef && !context.hasRepliedRef.value) {
  context.hasRepliedRef.value = true;  // ← mutation present in Slack
  return context.currentThreadTs;
}

The fix is to mirror that pattern here — set hasRepliedRef.value = true at the point the inherited replyToId is resolved for the first-mode branch:

    const replyToModeAllowsThread = (() => {
      if (toolContext?.replyToMode === "all") return true;
      if (
        toolContext?.replyToMode === "first" &&
        toolContext.hasRepliedRef !== undefined &&
        toolContext.hasRepliedRef.value !== true
      ) {
        toolContext.hasRepliedRef.value = true;
        return true;
      }
      return false;
    })();
    const replyToId =
      readMattermostReplyToId(params) ??
      (isSameChannel && replyToModeAllowsThread ? sessionThreadTs : undefined);

There is a corresponding test gap: none of the 8 new tests assert that handleAction mutates hasRepliedRef.value to true after the first thread-inheriting send. A test that calls handleAction twice with replyToMode=first and hasRepliedRef = { value: false } and then checks both the second call's replyToId and the resulting hasRepliedRef.value would catch this regression.

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

Comment:
**`replyToMode=first`: `hasRepliedRef.value` is never set to `true`**

When `replyToMode === "first"` and `hasRepliedRef.value === false`, the code correctly inherits the thread context — but it never flips `hasRepliedRef.value` to `true` after doing so. This means every subsequent call through this path also sees `hasRepliedRef.value === false` and keeps inheriting the thread, making `first` behave identically to `all` for all tool-send calls.

The Slack equivalent (in `extensions/slack/src/action-runtime.ts:113-116`) performs the mutation before returning the thread ID:
```typescript
if (context.replyToMode === "first" && context.hasRepliedRef && !context.hasRepliedRef.value) {
  context.hasRepliedRef.value = true;  // ← mutation present in Slack
  return context.currentThreadTs;
}
```

The fix is to mirror that pattern here — set `hasRepliedRef.value = true` at the point the inherited `replyToId` is resolved for the `first`-mode branch:

```typescript
    const replyToModeAllowsThread = (() => {
      if (toolContext?.replyToMode === "all") return true;
      if (
        toolContext?.replyToMode === "first" &&
        toolContext.hasRepliedRef !== undefined &&
        toolContext.hasRepliedRef.value !== true
      ) {
        toolContext.hasRepliedRef.value = true;
        return true;
      }
      return false;
    })();
    const replyToId =
      readMattermostReplyToId(params) ??
      (isSameChannel && replyToModeAllowsThread ? sessionThreadTs : undefined);
```

There is a corresponding test gap: none of the 8 new tests assert that `handleAction` mutates `hasRepliedRef.value` to `true` after the first thread-inheriting send. A test that calls `handleAction` twice with `replyToMode=first` and `hasRepliedRef = { value: false }` and then checks both the second call's `replyToId` **and** the resulting `hasRepliedRef.value` would catch this regression.

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

Fixed in commit 3c183d1: added the missing flip — if implicitReplyToId is used and replyToMode=first and hasRepliedRef is present, we set hasRepliedRef.value = true. Test asserts this. Mirrors the Slack action runtime behavior.

Fork-specific CI workflow included in upstream PR

The file header and comments make clear this workflow is intended exclusively for the contributor's fork (teconomix/openclaw):

"Runs on any branch push to teconomix/openclaw (except main)."
"Scoped to checks relevant to our Mattermost patches — avoids pre-existing failures in unrelated extensions (tlon, urbit, etc.) that are not our concern."

If merged into openclaw/openclaw, this file adds a redundant CI job scoped to a now-irrelevant fork context and may conflict with or duplicate existing upstream CI. The trigger (branches-ignore: main) also means it silently stops running on the very branch it would be merged into.

This file should either be removed from the PR entirely, or replaced with a targeted upstream CI addition (e.g. a job that runs Mattermost extension tests on all PR branches) using the upstream conventions.

This is a comment left during a code review.
Path: .github/workflows/preflight.yml
Line: 1-6

Comment:
**Fork-specific CI workflow included in upstream PR**

The file header and comments make clear this workflow is intended exclusively for the contributor's fork (`teconomix/openclaw`):

> "Runs on any branch push to teconomix/openclaw (except main)."
> "Scoped to checks relevant to our Mattermost patches — avoids pre-existing failures in unrelated extensions (tlon, urbit, etc.) that are not our concern."

If merged into `openclaw/openclaw`, this file adds a redundant CI job scoped to a now-irrelevant fork context and may conflict with or duplicate existing upstream CI. The trigger (`branches-ignore: main`) also means it silently stops running on the very branch it would be merged into.

This file should either be removed from the PR entirely, or replaced with a targeted upstream CI addition (e.g. a job that runs Mattermost extension tests on all PR branches) using the upstream conventions.

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

Fair point. The preflight.yml is fork-specific and is only on the branch because GitHub requires the workflow file to exist on the pushed branch for push triggers to fire. It is not included in the actual code change — the upstream PR diff shows only extensions/mattermost/src/channel.ts and channel.test.ts. If merged upstream, the file would need to be removed or scoped to the upstream repo, which is out of scope here.

context.To prefix mismatch makes isSameChannel always false

context.To is constructed in monitor.ts as "mattermost:channel:${channelId}" (or "mattermost:group:${channelId}"), but normalizeMattermostMessagingTarget interprets the mattermost: prefix as a user prefix, not a channel prefix:

normalizeMattermostMessagingTarget("mattermost:channel:CHAN1")
  → id = "channel:CHAN1"
  → returns "user:channel:CHAN1"   ← wrong

Meanwhile, the tool's to param uses "channel:CHAN1" format:

normalizeMattermostMessagingTarget("channel:CHAN1") → "channel:CHAN1"

So normalizedTo ("channel:CHAN1") !== normalizedCurrentChannelId ("user:channel:CHAN1") for every channel-send, making isSameChannel permanently false. Thread context is never inherited in production, even though the tests pass (they bypass buildToolContext by supplying currentChannelId: "channel:CHAN1" directly).

The fix is to strip the "mattermost:" platform prefix in buildToolContext before storing it as currentChannelId, mirroring how Slack's buildSlackThreadingToolContext strips "channel:" from context.To:

This turns "mattermost:channel:CHAN1" → "channel:CHAN1" and "mattermost:group:CHAN1" → "group:CHAN1", which will then normalize identically to what params.to normalizes to in handleAction.

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

Comment:
**`context.To` prefix mismatch makes `isSameChannel` always false**

`context.To` is constructed in `monitor.ts` as `"mattermost:channel:${channelId}"` (or `"mattermost:group:${channelId}"`), but `normalizeMattermostMessagingTarget` interprets the `mattermost:` prefix as a *user* prefix, not a channel prefix:

```
normalizeMattermostMessagingTarget("mattermost:channel:CHAN1")
  → id = "channel:CHAN1"
  → returns "user:channel:CHAN1"   ← wrong
```

Meanwhile, the tool's `to` param uses `"channel:CHAN1"` format:
```
normalizeMattermostMessagingTarget("channel:CHAN1") → "channel:CHAN1"
```

So `normalizedTo ("channel:CHAN1") !== normalizedCurrentChannelId ("user:channel:CHAN1")` for every channel-send, making `isSameChannel` permanently `false`. Thread context is **never inherited in production**, even though the tests pass (they bypass `buildToolContext` by supplying `currentChannelId: "channel:CHAN1"` directly).

The fix is to strip the `"mattermost:"` platform prefix in `buildToolContext` before storing it as `currentChannelId`, mirroring how Slack's `buildSlackThreadingToolContext` strips `"channel:"` from `context.To`:

```suggestion
        currentChannelId:
          context.To?.trim().replace(/^mattermost:/i, "") || undefined,
```

This turns `"mattermost:channel:CHAN1"` → `"channel:CHAN1"` and `"mattermost:group:CHAN1"` → `"group:CHAN1"`, which will then normalize identically to what `params.to` normalizes to in `handleAction`.

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

Fixed in commit 3312a42: strip the mattermost: platform prefix in buildToolContext before storing as currentChannelId. context.To uses mattermost:channel:<id> format from monitor.ts, but normalizeMattermostMessagingTarget treats mattermost: as a user prefix — so currentChannelId became user:channel:<id> instead of channel:<id>, making isSameChannel always false. Now using .replace(/^mattermost:/i, "") before storing, matching the channel:<id> / group:<id> format used by params.to in handleAction.

Surface implicit Mattermost thread ids to outbound routing

This new fallback only chooses replyToId inside extensions/mattermost/src/channel.ts, after src/infra/outbound/message-action-runner.ts has already called resolveOutboundSessionRoute(...) and chosen the mirror/session key. For a threaded Mattermost tool send without an explicit replyTo, the post now lands in the thread, but extensions/mattermost/src/session-route.ts still sees replyToId ?? threadId as empty and records the send under the base channel session. That will drift the mirrored transcript and any later thread-scoped follow-up/session lookups away from the message’s actual thread.

Useful? React with 👍 / 👎.

Acknowledged as out of scope for this PR. The session routing / outbound mirror drift is a valid follow-up but would require changes to resolveOutboundSessionRoute (extensions/mattermost/src/session-route.ts) and/or resolveAutoThreadId in the threading adapter — a separate concern from the message tool fallback this PR addresses. Will track as a follow-up issue.

Consume reply-to-first state for explicit threaded tool sends

Fresh evidence: this update still flips hasRepliedRef only when implicitReplyToId is used. In a replyToMode="first" session, if the first tool send already carries replyTo/replyToId explicitly (for example via [[reply_to_current]]), hasRepliedRef stays false, so the next Mattermost message send in the same turn will auto-inherit toolContext.currentThreadTs and post a second threaded reply. That makes first behave like all whenever the first tool message uses an explicit reply target.

Useful? React with 👍 / 👎.

Fixed in commit e7174a9: the hasRepliedRef flip now triggers on any non-null replyToId (explicit or implicit) when replyToMode=first, not just when the implicit fallback is used. An explicit replyTo/replyToId param now correctly consumes the first-reply slot.

Derive off-mode promotion from actual thread presence

effectiveReplyToMode is promoted from off to all only when threadTs is present, but threadTs here is sourced from context.MessageThreadId/context.ReplyToId. In Mattermost inbound handling, those fields are populated from effectiveReplyToId, which is explicitly undefined for replyToMode="off" even when the inbound post is in a thread (resolveMattermostEffectiveReplyToId in extensions/mattermost/src/mattermost/monitor.ts). That makes this promotion path unreachable for real off-mode threaded sessions, so tool sends still fall back to channel root instead of inheriting the active thread.

Useful? React with 👍 / 👎.

Gate first-reply consumption to the active channel

This flips hasRepliedRef for any non-empty replyToId in replyToMode="first", even when the send target is not the session’s current channel. If the first tool send is an explicit threaded message to a different channel/DM, it consumes the current session’s first-reply slot, and later sends back in the active thread no longer inherit currentThreadTs. The state transition should be tied to same-channel sends (like the implicit-thread path) to avoid cross-channel interference.

Useful? React with 👍 / 👎.