fix(msteams): make resolveMSTeamsRouteSessionKey idempotent against pre-suffixed bases (#66771) by harrisali0101 · Pull Request #78850 · openclaw/openclaw

harrisali0101 · 2026-05-07T08:13:38Z

Summary

Make resolveMSTeamsRouteSessionKey idempotent against pre-suffixed session keys by stripping any trailing (:thread:<id>)+ segments from baseSessionKey before delegating to resolveThreadSessionKeys. Closes #66771.

This is defense-in-depth at the helper boundary, complementary to (not a substitute for) the larger handler refactor in #59294. Even after that or a routing-cache clone-on-miss fix lands, making this helper itself idempotent prevents future caller-hygiene regressions from re-introducing the bug at any caller site.

Problem

Per #66771, msteams session lane keys can land in malformed shape:

agent:main:msteams:channel:19:<channel>@thread.tacv2:thread:OLD:thread:NEW

instead of the well-formed

agent:main:msteams:channel:19:<channel>@thread.tacv2:thread:NEW

Same-thread inbound turns split across distinct lanes; unrelated threads can collapse onto the same malformed key under the right cache state. Observed on @openclaw/msteams 2026.5.5.

Root cause

extensions/msteams/src/monitor-handler/message-handler.ts:489 reassigns route.sessionKey = resolveMSTeamsRouteSessionKey({ baseSessionKey: route.sessionKey, … }). The route object came from core.channel.routing.resolveAgentRoute(...). On a cache miss at src/routing/resolve-route.ts:680-696, the resolver creates the route, stores it in routeCache.set(routeCacheKey, route), and returns the same object reference (line 696: return route;, no clone). The cache hit path at line 650 returns { ...cachedRoute } (shallow clone) — but a shallow clone of an already-mutated cached object still carries the mutated sessionKey. So once a turn writes route.sessionKey = "…:thread:A" after a miss, the cached reference now has that suffix; subsequent hits return shallow copies that still include it; resolveMSTeamsRouteSessionKey then appends the new thread id on top, producing the mixed key.

The helper currently has no defense — it forwards params.baseSessionKey straight into resolveThreadSessionKeys (extensions/msteams/src/monitor-handler/thread-session.ts:12), which appends without any pre-existing-suffix check (src/routing/session-key.ts:251-253).

Approach

Three plausible fault domains; this PR takes (3):

	Where	Status / trade-off
1	Don't mutate `route.sessionKey` in the handler	Already attempted by #59294 (open since 2026-04-01, large surface).
2	Clone on cache-miss return	Touches `src/routing/`, broader review weight.
3	Idempotent strip-and-append in the msteams helper	This PR. Surgical, msteams-only.

Approach (3) doesn't preempt either of the other fixes — it's defensive at a different layer. After #59294 or a cache-clone fix lands, the helper still being idempotent is cheap insurance against future regressions at any caller of resolveMSTeamsRouteSessionKey.

Change

+const TRAILING_THREAD_SUFFIX = /(?::thread:[^:]+)+$/;
+
 export function resolveMSTeamsRouteSessionKey(params: { … }): string {
   const channelThreadId = params.isChannel
     ? (params.conversationMessageId ?? params.replyToId ?? undefined)
     : undefined;
+  const cleanBase = params.baseSessionKey.replace(TRAILING_THREAD_SUFFIX, "");
   return resolveThreadSessionKeys({
-    baseSessionKey: params.baseSessionKey,
+    baseSessionKey: cleanBase,
     threadId: channelThreadId,
-    parentSessionKey: channelThreadId ? params.baseSessionKey : undefined,
+    parentSessionKey: channelThreadId ? cleanBase : undefined,
   }).sessionKey;
}

Thread ids in Teams are timestamps/uuids — never contain : — so the segment boundary in the regex is unambiguous. The + covers pathological doubly-suffixed keys that may already exist in caches/persisted sessions from prior runs.

Tests

Adds four regression cases under a new idempotency against pre-suffixed bases (#66771) describe block in message-handler.thread-session.test.ts:

Already-thread-suffixed base + new thread → exactly one :thread:<new> suffix.
Doubly-thread-suffixed (already-malformed) base → collapsed onto the current thread.
Same-thread re-application is a fixed point.
Stale thread suffix + top-level inbound → bare base.

All existing tests pass unchanged. (Unit test runs are supplementary per the maintainers' proof gate; live behavioral evidence below is the load-bearing artifact.)

Real behavior proof

Behavior or issue addressed: Microsoft Teams channel-thread session lane keys could compound into :thread:OLD:thread:NEW or duplicate :thread:X:thread:X suffixes when a previously suffixed route session key was passed back through the Teams helper. This patch strips stale trailing Teams thread suffixes before appending the current thread id.
Real environment tested: macOS 24.3.0 (Darwin), Node 25.2.1, OpenClaw gateway on the standard local port, @openclaw/msteams 2026.5.5 patched locally with this helper change, Microsoft Teams Bot Framework channel-thread inbound activities in a Posts-style channel.
Exact steps or command run after this patch: Inspected the live ~/.openclaw/agents/main/sessions/sessions.json lane store, patched the local @openclaw/msteams bundle with this PR's helper change, restarted the gateway, confirmed the patched bundle was loaded, then ran node /tmp/verify-66771.mjs against the exact malformed lane keys captured from the live runtime store.
Evidence after fix: Copied terminal output from the patched local OpenClaw / msteams setup using live redacted Teams lane keys:

Pre-fix lane keys observed in ~/.openclaw/agents/main/sessions/sessions.json:
[thread suffixes: 0]                 agent:main:msteams:channel:19:<channel>@thread.tacv2
[thread suffixes: 1]                 agent:main:msteams:channel:19:<channel>@thread.tacv2:thread:<thread-root-A>
[thread suffixes: 2] <- MALFORMED    agent:main:msteams:channel:19:<channel>@thread.tacv2:thread:<thread-root-A>:thread:<thread-root-B>
[thread suffixes: 1]                 agent:main:msteams:channel:19:<channel>@thread.tacv2:thread:<thread-root-B>
[thread suffixes: 2] <- MALFORMED    agent:main:msteams:channel:19:<channel>@thread.tacv2:thread:<thread-root-B>:thread:<thread-root-B>

$ node /tmp/verify-66771.mjs

PASS MALFORMED #1 (live sessions.json): same-id double suffix
    in:  agent:main:msteams:channel:19:<channel>@thread.tacv2:thread:<thread-root-B>:thread:<thread-root-B>
    out: agent:main:msteams:channel:19:<channel>@thread.tacv2:thread:<thread-root-B>
PASS MALFORMED #2 (live sessions.json): OLD/NEW double suffix, exact #66771 shape
    in:  agent:main:msteams:channel:19:<channel>@thread.tacv2:thread:<thread-root-A>:thread:<thread-root-B>
    out: agent:main:msteams:channel:19:<channel>@thread.tacv2:thread:<thread-root-B>
PASS control: clean base + new thread
    in:  agent:main:msteams:channel:19:<channel>@thread.tacv2
    out: agent:main:msteams:channel:19:<channel>@thread.tacv2:thread:<thread-root-B>
PASS stale thread suffix + top-level inbound -> bare base
    in:  agent:main:msteams:channel:19:<channel>@thread.tacv2:thread:<thread-root-A>
    out: agent:main:msteams:channel:19:<channel>@thread.tacv2
PASS DM bypass - strip is a no-op when isChannel=false
    in:  agent:main:msteams:direct:<aad-object-id>
    out: agent:main:msteams:direct:<aad-object-id>

5/5 cases pass

Observed result after fix: Every malformed live runtime key normalizes to a single current-thread suffix; clean channel bases still append the requested thread; stale thread suffixes on top-level inbound messages collapse back to the bare channel base; direct chats bypass unchanged.
What was not tested: A live fresh-room Teams reproduction through the Bot Framework webhook was not rerun end-to-end. The proof uses exact malformed lane keys captured from the runtime sessions.json store after loading this helper patch into the local msteams bundle. Shared src/routing/resolve-route.ts cache cloning and the larger handler mutation refactor in fix(msteams): isolate thread sessions, outbound targeting, and attachment resolution #59294 are intentionally out of scope for this helper-level fix.

Scope and risks

1 source file changed (thread-session.ts, +14 / -2).
1 test file extended (message-handler.thread-session.test.ts, +55 / 0).
No public API change. No config schema change. No behavior change for clean-base callers.
Doesn't modify shared src/routing/ surface or any CODEOWNERS-restricted file.
Idempotent for already-clean keys: regex matches nothing, strip is a no-op.

Checklist

American English
Single-purpose change (one bug, one helper, one test surface)
No mixed concerns (no refactors / no formatting drive-bys)
No edits to CODEOWNERS-restricted files
References the existing issue and the related still-open PR
Real-behavior proof from a live OpenClaw setup with redacted runtime data (above)

clawsweeper · 2026-05-07T08:17:03Z

Codex review: needs maintainer review before merge.

Summary
This PR strips trailing Teams thread suffixes before resolving channel-thread session keys, adds regression coverage, and records the fix in the changelog.

Reproducibility: yes. at source level: current main mutates the Teams route session key and then passes a potentially pre-suffixed base into a helper that appends another thread suffix. The PR body adds live malformed lane-key evidence and terminal output showing the patched helper normalizes those keys.

Real behavior proof
Sufficient (terminal): The PR body includes redacted live malformed session keys and copied terminal verifier output from a patched local Teams bundle showing after-fix normalized keys.

Next step before merge
No ClawSweeper repair job is needed; the prior changelog finding is fixed and the remaining action is normal maintainer merge gating after CI.

Security
Cleared: The diff is limited to a Teams helper, focused tests, and a changelog line, with no dependency, workflow, permission, secret-handling, script, or package-resolution changes.

Review details

Best possible solution:

Land the Teams-owned helper idempotency fix after required CI completes, while keeping broader handler or route-cache refactors tracked separately in #59294.

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

Yes, at source level: current main mutates the Teams route session key and then passes a potentially pre-suffixed base into a helper that appends another thread suffix. The PR body adds live malformed lane-key evidence and terminal output showing the patched helper normalizes those keys.

Is this the best way to solve the issue?

Yes, the helper-level strip-and-append fix is the narrowest Teams-owned repair for the reported malformed lane keys without changing shared routing cache semantics. A shared cache clone fix can remain a separate follow-up if maintainers want it.

What I checked:

Current main compounds pre-suffixed bases: On current main, resolveMSTeamsRouteSessionKey forwards params.baseSessionKey directly into resolveThreadSessionKeys, so a base already ending in :thread:<old> can receive another thread suffix. (extensions/msteams/src/monitor-handler/thread-session.ts:12, 419b6e8993e4)
Shared thread helper appends without stripping: resolveThreadSessionKeys appends :thread:${normalizedThread} to the supplied base when a thread id is present and has no existing-suffix normalization. (src/routing/session-key.ts:251, 419b6e8993e4)
Mutation and cache path matches the report: The Teams handler mutates route.sessionKey, while the route resolver caches and returns the same route object on miss and only shallow-clones on hit, matching the stale pre-suffixed-base explanation. (extensions/msteams/src/monitor-handler/message-handler.ts:489, 419b6e8993e4)
Prepared head fixes the helper boundary: The PR adds TRAILING_THREAD_SUFFIX, derives cleanBase, and passes that clean base to resolveThreadSessionKeys and parentSessionKey. (extensions/msteams/src/monitor-handler/thread-session.ts:24, 0feaddbc142b)
Regression coverage exercises malformed shapes: The added tests cover old/new double suffixes, already-malformed double suffixes, same-thread idempotency, and top-level stale suffix stripping. (extensions/msteams/src/monitor-handler/message-handler.thread-session.test.ts:83, 0feaddbc142b)
Changelog blocker is repaired: The Teams fix bullet is now under Unreleased ### Fixes, addressing the earlier ClawSweeper finding about placement. (CHANGELOG.md:187, 0feaddbc142b)

Likely related people:

steipete: GitHub path history and blame point the current Teams thread-session helper, the handler route mutation area, and the shared session/routing helpers primarily to Peter Steinberger's recent work. (role: introduced behavior and recent maintainer; confidence: high; commits: bd49117a503c, ef6679843385, 42d73fd955af; files: extensions/msteams/src/monitor-handler/thread-session.ts, extensions/msteams/src/monitor-handler/message-handler.ts, src/routing/session-key.ts)
BradGroux: Brad rebased and prepared the PR head, moved the changelog entry into Fixes, reported focused test/build/check verification, and tracks this PR in the Microsoft issues/PRs maintainer tracker. (role: recent maintainer and likely follow-up owner; confidence: medium; commits: 94edbc782aef, 0feaddbc142b; files: CHANGELOG.md, extensions/msteams/src/monitor-handler/thread-session.ts, extensions/msteams/src/monitor-handler/message-handler.thread-session.test.ts)
vignesh07: Vignesh has recent merged history in resolveAgentRoute, which is relevant if maintainers choose to pursue the broader shared route-cache clone path separately from this Teams helper fix. (role: adjacent route resolver maintainer; confidence: low; commits: c260e207b299; files: src/routing/resolve-route.ts)

Remaining risk / open question:

The proof uses live malformed session keys and patched helper output rather than a fresh end-to-end Teams Bot Framework room reproduction.
One Critical Quality check was still in progress when inspected; merge should still follow required CI and branch-protection completion.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 419b6e8993e4.

BradGroux · 2026-05-08T08:56:02Z

Maintainer prep update for #78850 / #66771.

What changed:

Rebasing the PR onto current origin/main.
Added the required CHANGELOG.md entry with PR/issue attribution and thanks to @harrisali0101.

Prepared head:

Before: 226881952addda8ba22a624bbb00395d50d40a8d
After: 3268f499083d40c9f9f97f960a4ec4d3cde06cc5

Verification run locally after the final rebase:

pnpm test -- extensions/msteams/src/monitor-handler/message-handler.thread-session.test.ts passed: 10 tests.
pnpm build passed.
pnpm check passed.

Full pnpm test note:

I started the full suite through the maintainer gates, but stopped it after it had already collected unrelated failures outside this PR's Teams helper surface. The failures seen were in gateway server auth/config tests, agents fallback/harness tests, and plugin owner registry tests. The Teams-focused regression shard was green before and after the final rebase.

Fresh CI is now running on the prepared head.

BradGroux · 2026-05-08T09:29:24Z

Maintainer follow-up for #78850 / #66771.

ClawSweeper correctly caught that the changelog entry was under ### Changes; I moved it to the Unreleased ### Fixes section.

Prepared head:

Before: 3268f499083d40c9f9f97f960a4ec4d3cde06cc5
After: 0feaddbc142b60c461f28cee758b461ceae11e51

Verification after rebasing onto current origin/main:

pnpm test -- extensions/msteams/src/monitor-handler/message-handler.thread-session.test.ts passed: 10 tests.
pnpm build passed.
pnpm check passed.

Fresh CI is running on the updated prepared head.

…re-suffixed bases (openclaw#66771) When the inbound message handler reuses a `route.sessionKey` that was already thread-qualified by a prior turn (e.g. cached resolve-route hit, or the cache-miss return at src/routing/resolve-route.ts:696 whose object reference is then mutated in-place by message-handler.ts:489), naively appending the current thread id compounds into `…:thread:OLD:thread:NEW` and routes the turn to a malformed lane that splits same-thread context across turns. Strip any trailing `:thread:<id>+` segments from the base before delegating to resolveThreadSessionKeys. Thread ids never contain ':' so the segment boundary is unambiguous; the '+' covers pathological doubly-suffixed bases already in caches/persisted sessions from prior runs. This is defense-in-depth at the helper level, complementary to the larger handler refactor in openclaw#59294 (still open since 2026-04-01). Even after that or a routing-cache fix lands, making the helper itself idempotent prevents any future caller hygiene regression from re-introducing the bug. Adds four regression tests under a new "idempotency against pre-suffixed bases" describe block in message-handler.thread-session.test.ts: - already-thread-suffixed base + new thread → single clean suffix - doubly-thread-suffixed (already-malformed) base → collapsed to one - same-thread re-application is a fixed point - stale thread suffix + top-level inbound → bare base Closes openclaw#66771.

) Normalize pre-thread-qualified Teams route session keys before deriving channel-thread lanes so cached route reuse cannot create malformed mixed thread sessions. Co-authored-by: Harris Ali <[email protected]>

openclaw-barnacle Bot added channel: msteams Channel integration: msteams size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 7, 2026

clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 7, 2026

openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 7, 2026

BradGroux mentioned this pull request May 8, 2026

WORKING: All Microsoft Issues and PRs (refresh) #74163

Draft

BradGroux force-pushed the fix/msteams-mixed-thread-session-key-66771 branch from 2268819 to 3268f49 Compare May 8, 2026 08:55

clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 8, 2026

openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 8, 2026

BradGroux force-pushed the fix/msteams-mixed-thread-session-key-66771 branch from 3268f49 to 0feaddb Compare May 8, 2026 09:29

clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 8, 2026

openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 8, 2026

BradGroux added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 11, 2026

harrisali0101 and others added 3 commits May 11, 2026 05:26

MS Teams: document thread session key normalization

4e1c533

MS Teams: move changelog entry to section end

0403c35

BradGroux force-pushed the fix/msteams-mixed-thread-session-key-66771 branch from 0feaddb to 0403c35 Compare May 11, 2026 10:27

BradGroux merged commit 39cbe94 into openclaw:main May 11, 2026
104 checks passed

github-actions Bot mentioned this pull request May 11, 2026

📡 Upstream Digest — 2026-05-11 10:54 UTC curtismercier/openclaw-mods#829

Open

clawsweeper Bot mentioned this pull request May 11, 2026

fix(msteams): isolate thread sessions, outbound targeting, and attachment resolution #59294

Closed

15 tasks

xjodoin mentioned this pull request May 12, 2026

[Feature]: MSTeams channel-bound agents need opt-out from per-thread sessions #81084

Open

DanBotero mentioned this pull request May 26, 2026

Hotfix Teams thread participation context Botero-Labs/openclaw#18

Merged

clawsweeper Bot mentioned this pull request Jul 2, 2026

[Bug]: Teams app removal/re-add retains prior DM session history #99054

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(msteams): make resolveMSTeamsRouteSessionKey idempotent against pre-suffixed bases (#66771)#78850

fix(msteams): make resolveMSTeamsRouteSessionKey idempotent against pre-suffixed bases (#66771)#78850
BradGroux merged 3 commits into
openclaw:mainfrom
harrisali0101:fix/msteams-mixed-thread-session-key-66771

harrisali0101 commented May 7, 2026 •

edited by BradGroux

Loading

Uh oh!

clawsweeper Bot commented May 7, 2026 •

edited

Loading

Uh oh!

BradGroux commented May 8, 2026

Uh oh!

BradGroux commented May 8, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

harrisali0101 commented May 7, 2026 • edited by BradGroux Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Problem

Root cause

Approach

Change

Tests

Real behavior proof

Scope and risks

Checklist

Uh oh!

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

Uh oh!

Uh oh!

BradGroux commented May 8, 2026

Uh oh!

BradGroux commented May 8, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

harrisali0101 commented May 7, 2026 •

edited by BradGroux

Loading

clawsweeper Bot commented May 7, 2026 •

edited

Loading