Codex review: needs changes before merge. Reviewed July 10, 2026, 2:55 PM ET / 18:55 UTC.

Summary
The PR dispatches Microsoft Teams removal lifecycle events, removes matching cached conversation references, rotates routed sessions, and detects observable personal-DM conversation-id changes.

PR surface: Source +423, Tests +575. Total +998 across 8 files.

Reproducibility: yes. Create a matching Teams session entry with updatedAt: 0 and provider-owned binding metadata, trigger the removal handler, and observe that the PR skips it even though current session resolution still treats it as fresh.

Review metrics: 1 noteworthy metric.

• Stored session reset paths: 1 added. The new replacement path clears provider routing metadata but currently excludes matching entries already timestamped stale.

Stored data model
Persistent data-model change detected: migration/backfill/repair: extensions/msteams/src/monitor-handler/lifecycle-handler.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #99054
Summary: This PR is the stronger candidate fix for the canonical Teams lifecycle issue but needs one correctness repair; the competing stale-only PR is not a safe co-landing path.

Members:

• canonical: [Bug]: Teams app removal/re-add retains prior DM session history #99054 - The issue documents the exact Teams app removal and re-add context-retention behavior addressed here.
• superseded: fix(msteams): handle bot removal and uninstallation to mark sessions stale #100350 - That PR uses hard-coded keys and stale-only updates that do not clear provider-owned session freshness, while both PRs register the same lifecycle events.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦞 diamond lobster
Patch quality: 🦐 gold shrimp
Result: needs maintainer review before merge.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

• [P2] Rotate zero-timestamp provider-bound entries and add focused regression coverage.

Risk before merge

• [P1] A matching Teams entry already marked updatedAt: 0 can retain its old session identifier and provider binding after app removal, allowing prior context to resume.
• [P1] The PR intentionally preserves transcript files while severing active session and proactive-send linkage; maintainers still need to approve that retention boundary.
• [P1] The overlapping fix(msteams): handle bot removal and uninstallation to mark sessions stale #100350 handles the same events with a weaker stale-only path and must not land alongside this implementation.

Maintainer options:

1. Repair stale provider-bound entries (recommended)
   Rotate matching entries even when updatedAt is already zero and add focused provider-binding regression coverage.
2. Accept incomplete lifecycle isolation
   Merge knowing a previously stale provider-bound Teams session can still resume old context after removal.
3. Pause for retention policy
   Hold the PR if maintainers are not ready to define what transcript data survives an observable Teams uninstall.

@clawsweeper automerge

Special instructions:
Ensure Teams lifecycle rotation replaces matching entries whose updatedAt is already 0 when provider-owned metadata remains, and add focused regression coverage.

Next step before merge

• [P2] One narrow mechanical P1 repair is suitable for automation before maintainers make the separate transcript-retention decision.

Maintainer decision needed

• Question: After the stale-entry bypass is repaired, should observable Microsoft Teams app removal preserve transcript artifacts while severing active session context and cached proactive-send linkage?
• Rationale: The mechanical defect has a clear repair, but code correctness alone cannot choose OpenClaw's post-uninstall transcript-retention policy.
• Likely owner: steipete — The shared Teams conversation-store refactor and adjacent session-routing work provide the strongest available ownership signal for this retention decision.
• Options:
  • Retain transcripts, reset linkage (recommended): Rotate every matching session and remove cached Teams conversation references while preserving transcript artifacts.
  • Define deletion semantics first: Pause landing until app removal also specifies transcript deletion, archival, or another explicit retention policy.

Security
Needs attention: The stale provider-bound bypass leaves a concrete privacy-boundary gap in the proposed uninstall reset.

Review findings

• [P1] Rotate matching sessions even when updatedAt is already zero — extensions/msteams/src/monitor-handler/lifecycle-handler.ts:124-125

I pushed a follow-up that tightens the implementation and updates the proof packet.

What changed since the first draft:

• Teams app removal still handles installationUpdate action=remove|remove-upgrade and bot membersRemoved.
• The reset now rotates the stored session id, sets updatedAt: 0, and clears Teams route/delivery/origin metadata instead of only marking the existing entry stale.
• Personal installationUpdate action=add now acts as a reinstall boundary only if an active route-matched Teams DM session already exists. First-time installs remain effectively ignored because there is no prior session to rotate.
• Inbound personal DMs now check for a stored same-user/same-bot conversation reference with a different Teams conversation.id; if Teams exposes that boundary, OpenClaw removes the previous reference and rotates the DM session.

Live app-management removal proof:

baseline:
  synthetic marker sent before Teams app-management removal

remove lifecycle:
  type=installationUpdate action=remove conversationType=personal
  msteams lifecycle remove handled
  reason=installation-remove conversationRemoved=true sessionsReset=1

session state:
  previous active Teams DM session rotated to a new redacted session id
  updatedAt=0
  Teams route/delivery/origin metadata cleared

after re-add:
  Teams welcome card appeared
  verification prompt used a fresh session
  response did not recall the pre-removal marker

Live Remove chat history follow-up:

pre-clear:
  synthetic marker sent and acknowledged

user action:
  Teams Remove chat history
  then user opened the app/chat again and sent a verification prompt

observed:
  no installationUpdate add/remove
  no membersAdded/membersRemoved lifecycle boundary
  no msteams dm conversation boundary handled log
  same OpenClaw session id remained active
  response could recall the pre-clear marker

So this PR intentionally fixes the observable Teams app lifecycle boundaries and changed-conversation-id boundary. It does not claim to fix Teams local Remove chat history when Teams sends only a normal message in the same server-side conversation.

Validation:

pnpm test extensions/msteams/src/monitor-handler/lifecycle-handler.test.ts
  PASS 11 tests

pnpm test extensions/msteams/src/monitor-handler/message-handler.authz.test.ts
  PASS 18 tests

pnpm test extensions/msteams
  PASS 71 files, 1007 tests

pnpm run lint:extensions:bundled
  PASS

pnpm format:check extensions/msteams/src/monitor-handler/lifecycle-handler.ts extensions/msteams/src/monitor-handler/lifecycle-handler.test.ts extensions/msteams/src/monitor-handler/message-handler.ts
  PASS

git diff --check
  PASS

@clawsweeper re-review

🦞🧹
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.

Reviewed at 158e595. This looks correct and is the stronger of the two lifecycle fixes.

Confirmed the gap on main: extensions/msteams/src/monitor-handler.ts registers no onInstallationUpdate/onMembersRemoved, so a removed-then-re-added app resumes the prior session. Notably, just marking the entry stale (updatedAt: 0) is not enough on its own — src/agents/command/session.ts:389 sets skipImplicitExpiry when hasProviderOwnedSession is true (src/config/sessions/entry-freshness.ts:48), which keeps a provider-owned session "fresh" regardless of updatedAt. This PR's full replaceEntry down to { sessionId, updatedAt: 0 } in rotateMSTeamsSessions (extensions/msteams/src/monitor-handler/lifecycle-handler.ts:731) drops the provider binding and route/origin metadata, so freshness actually fails and the next message starts clean. Deriving the key from the same resolveAgentRoute used to write sessions keeps the reset keyed correctly by construction, and the compare-and-swap in the patchSessionEntry update (lifecycle-handler.ts:739) makes the rotation race-safe. Coverage of personal remove/remove-upgrade, reinstall-add gated on an existing session, channel/group bot removal plus :thread: sessions, and the DM conversation-id boundary all read well and are backed by tests.

Two non-blocking notes:

1. findStoredPersonalDmConversation (extensions/msteams/src/monitor-handler/lifecycle-handler.ts:791) largely re-implements findPreferredDmConversationByUserId (extensions/msteams/src/conversation-store-helpers.ts:56), already surfaced by the store as findPreferredDmByUserId (extensions/msteams/src/conversation-store-state.ts:190). Reusing that and layering the bot-id check would drop the duplicated personal/user filter + lastSeen sort.
2. handleMSTeamsDmConversationBoundary runs on every inbound DM and does a full conversationStore.list(), now ahead of the poll-vote/empty-body early returns. Fine for typical volumes, but worth confirming that's acceptable as a per-DM cost, or routing it through the existing preferred-DM lookup.

One process point for maintainers: #100350 targets the same two lifecycle events with a mark-stale-only, personal-DM-only approach and a hardcoded agent:<accountId>:msteams:direct:<userId> key. Merging both would double-register the handlers, and the mark-stale path is defeated by the provider-owned-session case above — so these should not both land.

Maintainer note: these existing PRs were opened from the Spinen organization fork, and I learned that GitHub's normal "allow edits from maintainers" flow may not work reliably for org-owned forks.

For future OpenClaw PRs, I'll open from my personal fork so maintainer edits work normally.

For this existing batch, I want to make collaboration as easy as possible. I've invited @steipete as a temporary Write collaborator on spinen/openclaw in case direct branch edits are useful. I'm also happy to grant temporary access to another specific maintainer, quickly apply/cherry-pick requested changes myself, or support a maintainer takeover PR if that is the fastest path.

My goal is to make these branches easy to finish, not to create extra process.

Superseded by #104690 from my personal fork, using the same published head commit. This move preserves my authorship while enabling maintainer edits and repository automation. Please continue review on #104690; this PR remains available for prior discussion, proof, and CI history.