msteams: extend MSTeamsAdapter and MSTeamsActivityHandler types; implement self() [AI] by sudie-codes · Pull Request #49929 · openclaw/openclaw

sudie-codes · 2026-03-18T16:54:44Z

Summary

Problem: MSTeamsAdapter lacked updateActivity/deleteActivity method signatures, blocking message edit/delete features from being typed correctly.
Problem: MSTeamsActivityHandler lacked onReactionsAdded/onReactionsRemoved registration methods, blocking reaction handling.
Problem: directory.self() always returned null, so the bot had no identity in directory lookups.
What changed: Extended both types with the missing method signatures; implemented self() to return the bot's AAD app ID from configured credentials.
What did NOT change: No runtime behavior changed — these are type-level additions and a minimal self() implementation. No new network calls, no auth changes.

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Related: msteams-feature-parity team plan item 3.0 (prerequisite for reactions, message edit, streaming)

User-visible / Behavior Changes

openclaw directory self --channel msteams will now return the bot's App ID instead of nothing.
Otherwise no user-visible changes; type extensions are internal contracts.

Security Impact (required)

New permissions/capabilities? No
Secrets/tokens handling changed? No — reads existing appId credential already stored
New/changed network calls? No
Command/tool execution surface changed? No
Data access scope changed? No

Repro + Verification

Environment

OS: macOS 24.4.0
Runtime: Node 22 / Bun
Integration/channel: MS Teams (msteams extension)

Steps

pnpm test -- extensions/msteams/src/channel.directory.test.ts
pnpm test -- extensions/msteams

Expected

self() returns { kind: "user", id: "<appId>", name: "<appId>" } when credentials are configured
self() returns null when credentials are absent

Actual

All 236 tests pass, including 2 new self() tests

Evidence

Failing test/log before + passing after

Test run (27 test files, 236 tests):

Test Files  27 passed (27)
     Tests  236 passed (236)
  Duration  9.43s

Human Verification (required)

Verified scenarios: self() with credentials returns bot appId; self() without credentials returns null; full msteams test suite passes
Edge cases checked: missing credentials, partial credentials (appPassword missing → returns null)
What you did not verify: live Teams integration (no test tenant available)

Review Conversations

I replied to or resolved every bot review conversation I addressed in this PR.
I left unresolved only the conversations that still need reviewer or maintainer judgment.

Compatibility / Migration

Backward compatible? Yes — additive type changes only
Config/env changes? No
Migration needed? No

Failure Recovery (if this breaks)

How to disable/revert: Revert the 4-file commit; no state is persisted
Known bad symptoms: None expected

Risks and Mitigations

Risk: self() returns appId (a UUID-like string) as both id and name. If downstream code expects a human-readable display name, the name will look like a GUID.
- Mitigation: This matches the same pattern as other unresolved identities; a follow-up can resolve the bot's display name via Graph API.

AI Disclosure

Mark as AI-assisted in the PR title or description
Note the degree of testing: fully tested (236 tests pass)
Confirm I understand what the code does
Tool: Claude Sonnet 4.6 via OpenClaw agent team

…ement self() - Add updateActivity/deleteActivity to MSTeamsAdapter - Add onReactionsAdded/onReactionsRemoved to MSTeamsActivityHandler - Implement directory self() to return bot identity from appId credential - Add tests for self() in channel.directory.test.ts

greptile-apps · 2026-03-18T16:58:27Z

Greptile Summary

This PR makes three additive changes to the msteams extension: implements directory.self() to return the bot's AAD App ID, and extends the MSTeamsAdapter and MSTeamsActivityHandler local type definitions with updateActivity/deleteActivity and onReactionsAdded/onReactionsRemoved respectively. All changes are non-breaking at runtime.

Key finding:

The new updateActivity and deleteActivity members are added as required (non-optional) properties on MSTeamsAdapter. Five existing test fixtures in messenger.test.ts and monitor-handler.file-consent.test.ts create MSTeamsAdapter-typed objects that omit these methods. Bun's test runner passes because it strips types at runtime, but a tsc --noEmit check would emit errors for each incomplete mock. This should be resolved either by marking the two new methods optional (updateActivity?, deleteActivity?) — which better matches the stated "additive, no behaviour change" goal — or by updating the affected test mocks.

Everything else looks correct:

self() correctly delegates to resolveMSTeamsCredentials, which handles both config and env-var credential paths, and returns null when credentials are absent.
onReactionsAdded/onReactionsRemoved follow the exact same pattern as the existing onMessage/onMembersAdded signatures.
The two new self() tests are well-structured and cover the intended contract.
The GUID-as-name limitation in self() is acknowledged in the PR description with a clear follow-up path.

Confidence Score: 3/5

Safe to merge after resolving the MSTeamsAdapter optional vs. required method question to avoid TypeScript compilation regressions in existing test mocks.
The self() implementation and the MSTeamsActivityHandler extensions are correct and well-tested. The only concrete issue is that updateActivity and deleteActivity are added as required properties, silently invalidating five existing MSTeamsAdapter mock objects in tests that were not updated as part of this PR. Bun's runtime-only test execution masks the TypeScript type error, but tsc will flag them. This is a straightforward fix (mark the two new properties optional, or patch the five mocks) but it needs to be done before merge to keep the type system consistent.
extensions/msteams/src/messenger.ts — new required properties may need to be optional; the corresponding test mocks in messenger.test.ts and monitor-handler.file-consent.test.ts (not in this diff) are now type-invalid.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: extensions/msteams/src/messenger.ts
Line: 63-64

Comment:
**New required methods break existing test mock types**

`updateActivity` and `deleteActivity` are added as **required** (non-optional) properties on `MSTeamsAdapter`, but several existing test fixtures create objects typed as `MSTeamsAdapter` without these methods. This will cause TypeScript type errors (even though Bun's test runner succeeds at runtime by stripping types):

- `messenger.test.ts:53–56` — `createNoopAdapter()`
- `messenger.test.ts:77–84` — `createFallbackAdapter()`
- `messenger.test.ts:190–198` — inline adapter in `"sends top-level messages via continueConversation…"` test
- `messenger.test.ts:364–369` — inline adapter in `"retries top-level sends on transient (5xx)"` test
- `monitor-handler.file-consent.test.ts:42–45` — inline adapter in `createDeps()`

Two options to fix this:

**Option A** — Mark the new methods optional (preferred, since no call-sites exist yet):
```suggestion
  updateActivity?: (context: unknown, activity: object) => Promise<void>;
  deleteActivity?: (context: unknown, reference: { activityId?: string }) => Promise<void>;
```

**Option B** — Add stub implementations in every affected test mock (more noise, but forces completeness):
```typescript
// In each mock factory / inline adapter, add:
updateActivity: async () => {},
deleteActivity: async () => {},
```

Option A keeps the change purely additive and non-breaking, which matches the stated goal ("additive type changes only"). Option B is better if call-sites are expected imminently and you want the mocks to stay honest.

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

_{Last reviewed commit: "msteams: extend MSTe..."}

greptile-apps · 2026-03-18T16:58:31Z

extensions/msteams/src/messenger.ts

+  updateActivity: (context: unknown, activity: object) => Promise<void>;
+  deleteActivity: (context: unknown, reference: { activityId?: string }) => Promise<void>;


New required methods break existing test mock types

updateActivity and deleteActivity are added as required (non-optional) properties on MSTeamsAdapter, but several existing test fixtures create objects typed as MSTeamsAdapter without these methods. This will cause TypeScript type errors (even though Bun's test runner succeeds at runtime by stripping types):

messenger.test.ts:53–56 — createNoopAdapter()

messenger.test.ts:77–84 — createFallbackAdapter()

messenger.test.ts:190–198 — inline adapter in "sends top-level messages via continueConversation…" test

messenger.test.ts:364–369 — inline adapter in "retries top-level sends on transient (5xx)" test

monitor-handler.file-consent.test.ts:42–45 — inline adapter in createDeps()

Two options to fix this:

Option A — Mark the new methods optional (preferred, since no call-sites exist yet):

Suggested change

updateActivity: (context: unknown, activity: object) => Promise<void>;

deleteActivity: (context: unknown, reference: { activityId?: string }) => Promise<void>;

updateActivity?: (context: unknown, activity: object) => Promise<void>;

deleteActivity?: (context: unknown, reference: { activityId?: string }) => Promise<void>;

Option B — Add stub implementations in every affected test mock (more noise, but forces completeness):

// In each mock factory / inline adapter, add: updateActivity: async () => {}, deleteActivity: async () => {},

Option A keeps the change purely additive and non-breaking, which matches the stated goal ("additive type changes only"). Option B is better if call-sites are expected imminently and you want the mocks to stay honest.

Prompt To Fix With AI

This is a comment left during a code review. Path: extensions/msteams/src/messenger.ts Line: 63-64 Comment: **New required methods break existing test mock types** `updateActivity` and `deleteActivity` are added as **required** (non-optional) properties on `MSTeamsAdapter`, but several existing test fixtures create objects typed as `MSTeamsAdapter` without these methods. This will cause TypeScript type errors (even though Bun's test runner succeeds at runtime by stripping types): - `messenger.test.ts:53–56` — `createNoopAdapter()` - `messenger.test.ts:77–84` — `createFallbackAdapter()` - `messenger.test.ts:190–198` — inline adapter in `"sends top-level messages via continueConversation…"` test - `messenger.test.ts:364–369` — inline adapter in `"retries top-level sends on transient (5xx)"` test - `monitor-handler.file-consent.test.ts:42–45` — inline adapter in `createDeps()` Two options to fix this: **Option A** — Mark the new methods optional (preferred, since no call-sites exist yet): ```suggestion updateActivity?: (context: unknown, activity: object) => Promise<void>; deleteActivity?: (context: unknown, reference: { activityId?: string }) => Promise<void>; ``` **Option B** — Add stub implementations in every affected test mock (more noise, but forces completeness): ```typescript // In each mock factory / inline adapter, add: updateActivity: async () => {}, deleteActivity: async () => {}, ``` Option A keeps the change purely additive and non-breaking, which matches the stated goal ("additive type changes only"). Option B is better if call-sites are expected imminently and you want the mocks to stay honest. How can I resolve this? If you propose a fix, please make it concise.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 410f2e8373

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-03-18T17:09:58Z

extensions/msteams/src/channel.ts

  },
  directory: createChannelDirectoryAdapter({
+    self: async ({ cfg }) => {
+      const creds = resolveMSTeamsCredentials(cfg.channels?.msteams);


Resolve self ID without requiring full Teams credentials

This self() path calls resolveMSTeamsCredentials, which requires a resolved appPassword and tenantId and can throw when channels.msteams.appPassword is an unresolved SecretRef. In that configuration, openclaw directory self --channel msteams now errors even though this lookup only needs the bot appId, so users with SecretRef-backed credentials can no longer use directory self reliably.

Useful? React with 👍 / 👎.

BradGroux

Clean type extensions. updateActivity/deleteActivity on the adapter + reaction handler types follow existing patterns. self() implementation is minimal and correct — returning null when credentials aren't configured is the right call.

Note: #49925 duplicates the self() implementation. This PR should land first so #49925 can rebase and drop its copy.

Minor: appId as display name is a GUID, a follow-up to resolve via Graph API would be nice.

✅ Approve.

…ement self() (openclaw#49929) - Add updateActivity/deleteActivity to MSTeamsAdapter - Add onReactionsAdded/onReactionsRemoved to MSTeamsActivityHandler - Implement directory self() to return bot identity from appId credential - Add tests for self() in channel.directory.test.ts

…ement self() (openclaw#49929) - Add updateActivity/deleteActivity to MSTeamsAdapter - Add onReactionsAdded/onReactionsRemoved to MSTeamsActivityHandler - Implement directory self() to return bot identity from appId credential - Add tests for self() in channel.directory.test.ts (cherry picked from commit 7c3af37)

* fix(msteams): resolve Graph API chat ID for DM file uploads (openclaw#49585) Fixes openclaw#35822 — Bot Framework conversation.id format is incompatible with Graph API /chats/{chatId}. Added resolveGraphChatId() to look up the Graph-native chat ID via GET /me/chats, cached in the conversation store. Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]> (cherry picked from commit 06845a1) * test: fix fetch mock typing (cherry picked from commit 0f43dc4) * fix: restore repo-wide gate after upstream sync (cherry picked from commit 14074d3) * test(msteams): align adapter doubles with interfaces (cherry picked from commit 5b7ae24) * test: tighten msteams regression assertions (cherry picked from commit c8a36c6) * test: dedupe msteams attachment redirects (cherry picked from commit 017c0dc) * MSTeams: move outbound session routing behind plugin boundary (cherry picked from commit 028f3c4) * fix: remove session-route.ts — depends on missing upstream infrastructure * test(msteams): cover graph helpers (cherry picked from commit 1ea2593) * fix(test): split msteams attachment helpers (cherry picked from commit 23c8af3) * test: share directory runtime helpers (cherry picked from commit 38b0986) * fix: stabilize build dependency resolution (openclaw#49928) * build: mirror uuid for msteams Add uuid to both the msteams bundled extension and the root package so the workspace build can resolve @microsoft/agents-hosting during tsdown while standalone extension installs also have the runtime dependency available. Regeneration-Prompt: | pnpm build failed because @microsoft/agents-hosting 1.3.1 requires uuid in its published JS but does not declare it in its package manifest. The msteams extension dynamically imports that package, and the workspace build resolves it from the root dependency graph. Mirror uuid into the root package for workspace builds and keep it in extensions/msteams/package.json so standalone plugin installs also resolve it. Update the lockfile to match the manifest changes. * build: prune stale plugin dist symlinks Remove stale dist and dist-runtime plugin node_modules symlinks before tsdown runs. These links point back into extension installs, and tsdown's clean step can traverse them on rebuilds and hollow out the active pnpm dependency tree before plugin-sdk declaration generation runs. Regeneration-Prompt: | pnpm build was intermittently failing in the plugin-sdk:dts phase after earlier build steps had already run. The symptom looked like missing root packages such as zod, ajv, commander, and undici even though a fresh install briefly fixed the problem. Investigate the build pipeline step by step rather than patching TypeScript errors. Confirm whether rebuilds mutate node_modules, identify the first step that does it, and preserve existing runtime-postbuild behavior. The key constraint is that dist and dist-runtime plugin node_modules links are intentional for runtime packaging, so do not remove that feature globally. Instead, make rebuilds safe by deleting only stale symlinks left in generated output before invoking tsdown, so tsdown cleanup cannot recurse back into the live pnpm install tree. Verify with repeated pnpm build runs. (cherry picked from commit 505d140) * test(msteams): cover store and live directory helpers (cherry picked from commit 55e0c63) * test(msteams): cover setup wizard status (cherry picked from commit 653d69e) * test: tighten msteams regression assertions (cherry picked from commit 689a734) * refactor: share teams drive upload flow (cherry picked from commit 6b04ab1) * test(msteams): cover routing and setup (cherry picked from commit 774a206) * msteams: extend MSTeamsAdapter and MSTeamsActivityHandler types; implement self() (openclaw#49929) - Add updateActivity/deleteActivity to MSTeamsAdapter - Add onReactionsAdded/onReactionsRemoved to MSTeamsActivityHandler - Implement directory self() to return bot identity from appId credential - Add tests for self() in channel.directory.test.ts (cherry picked from commit 7c3af37) * test(msteams): cover upload and webhook helpers (cherry picked from commit 7d11f6c) * msteams: fix sender allowlist bypass when route allowlist is configured (GHSA-g7cr-9h7q-4qxq) (openclaw#49582) When a route-level (teams/channel) allowlist was configured but the sender allowlist (allowFrom/groupAllowFrom) was empty, resolveSenderScopedGroupPolicy would downgrade the effective group policy from "allowlist" to "open", allowing any Teams user to interact with the bot. The fix: when channelGate.allowlistConfigured is true and effectiveGroupAllowFrom is empty, preserve the configured groupPolicy ("allowlist") rather than letting it be downgraded to "open". This ensures an empty sender allowlist with an active route allowlist means deny-all rather than allow-all. Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]> (cherry picked from commit 897cda7) * fix(msteams): batch multi-block replies into single continueConversation call (openclaw#29379) (openclaw#49587) Teams silently drops blocks 2+ when each deliver() opens its own continueConversation() call. Accumulate rendered messages across all deliver() calls and flush them together in markDispatchIdle(). On batch failure, retry each message individually so trailing blocks are not silently lost. Log a warning when any individual messages fail so flush failures are visible in logs. (cherry picked from commit 8b5eeba) * test(msteams): cover poll and file-card helpers (cherry picked from commit 8ff277d) * test: dedupe msteams consent auth fixtures (cherry picked from commit a9d8518) * refactor: share dual text command gating (cherry picked from commit b61bc49) * test: share msteams safe fetch assertions (cherry picked from commit d4d0091) * MSTeams: lazy-load runtime-heavy channel paths (cherry picked from commit da4f825) * fix(msteams): isolate probe test env credentials (cherry picked from commit e9078b3) * test: dedupe msteams policy route fixtures (cherry picked from commit f2300f4) * fix: fix remaining openclaw references in cherry-picked msteams files * fix: adapt cherry-picks for fork TS strictness * fix: revert cross-cutting refactors, keep msteams-specific changes only * fix: format cherry-picked files with oxfmt --------- Co-authored-by: sudie-codes <[email protected]> Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]> Co-authored-by: Peter Steinberger <[email protected]> Co-authored-by: Vincent Koc <[email protected]> Co-authored-by: Gustavo Madeira Santana <[email protected]> Co-authored-by: Josh Lehman <[email protected]>

openclaw-barnacle bot added channel: msteams Channel integration: msteams size: XS labels Mar 18, 2026

greptile-apps bot reviewed Mar 18, 2026

View reviewed changes

sudie-codes mentioned this pull request Mar 18, 2026

msteams: add message edit and delete support #49925

Merged

21 tasks

chatgpt-codex-connector bot reviewed Mar 18, 2026

View reviewed changes

BradGroux self-assigned this Mar 20, 2026

BradGroux approved these changes Mar 20, 2026

View reviewed changes

BradGroux mentioned this pull request Mar 20, 2026

fix(msteams): update FileConsentCard after user accepts upload #49580

Open

3 tasks

BradGroux merged commit 7c3af37 into openclaw:main Mar 20, 2026
35 of 44 checks passed

github-actions bot mentioned this pull request Mar 20, 2026

📡 Upstream Digest — 2026-03-20 16:34 UTC curtismercier/openclaw-mods#316

Open

alexey-pelykh mentioned this pull request Mar 23, 2026

Cherry-pick: MS Teams adapter (34 commits) remoteclaw/remoteclaw#1878

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

msteams: extend MSTeamsAdapter and MSTeamsActivityHandler types; implement self() [AI]#49929

msteams: extend MSTeamsAdapter and MSTeamsActivityHandler types; implement self() [AI]#49929
BradGroux merged 1 commit intoopenclaw:mainfrom
sudie-codes:feat/msteams-extend-adapter-types

sudie-codes commented Mar 18, 2026

Uh oh!

greptile-apps bot commented Mar 18, 2026

Uh oh!

greptile-apps bot Mar 18, 2026

Uh oh!

chatgpt-codex-connector bot left a comment

Uh oh!

chatgpt-codex-connector bot Mar 18, 2026

Uh oh!

BradGroux left a comment •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		updateActivity: (context: unknown, activity: object) => Promise<void>;
		deleteActivity: (context: unknown, reference: { activityId?: string }) => Promise<void>;

Uh oh!

Conversation

sudie-codes commented Mar 18, 2026

Summary

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

User-visible / Behavior Changes

Security Impact (required)

Repro + Verification

Environment

Steps

Expected

Actual

Evidence

Human Verification (required)

Review Conversations

Compatibility / Migration

Failure Recovery (if this breaks)

Risks and Mitigations

AI Disclosure

Uh oh!

greptile-apps bot commented Mar 18, 2026

Greptile Summary

Confidence Score: 3/5

Uh oh!

greptile-apps bot Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

BradGroux left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

BradGroux left a comment •

edited

Loading