fix(msteams): handle fileConsent/invoke callback for bot-to-user file upload (#55386) by sudie-codes · Pull Request #64087 · openclaw/openclaw

sudie-codes · 2026-04-10T04:52:38Z

Summary

Bot-to-user file uploads via openclaw message send --media were silently failing because the fileConsent/invoke callback was never handled across processes. The CLI sender registered the pending upload in its own memory, but the webhook receiver ran in a different process and never saw it.

Fix

New filesystem-backed pending upload store (pending-uploads-fs.ts) under <stateDir>/msteams-pending-uploads/ so CLI sender and webhook receiver share state
fileConsent/invoke handler: on accept → PUT file to upload URL + send FileInfoCard; on decline → drop pending upload
Graceful degradation if FS store is unavailable

Test plan

17 new tests (store primitives, accept/decline paths, cross-process, missing upload)
589 msteams tests passing

Fixes #55386

🤖 Generated with Claude Code

greptile-apps · 2026-04-10T04:57:00Z

Greptile Summary

This PR fixes a cross-process bug in the MSTeams fileConsent/invoke flow: the CLI message send --media path stored pending uploads in an in-memory Map, but the invoke callback arrived in the long-running monitor webhook process where that Map is empty. The fix introduces a filesystem-backed pending upload store under <stateDir>/msteams-pending-uploads/ with file-lock-protected index updates, TTL-based pruning, and a safety-first UUID path guard. The monitor handler now checks both stores (memory first, then FS) and cleans up from the correct one. Tests cover store primitives, cross-process round-trips, accept/decline paths, and cross-conversation security checks.

Confidence Score: 5/5

Safe to merge — the core fix is correct, tests are comprehensive, and remaining findings are P2 quality-only concerns.

No P0 or P1 issues found. The malformed-accept edge case (else branch conflation) and the missing isSafeUploadId guard on pruned keys are both P2 suggestions — neither affects the primary file-upload path or introduces a practical data-loss or security risk under normal Teams behaviour.

extensions/msteams/src/monitor-handler.ts (else branch logic) and extensions/msteams/src/pending-uploads-fs.ts (pruneExpired key validation)

Comments Outside Diff (1)

extensions/msteams/src/monitor-handler.ts, line 264-314 (link)

Malformed accept falls through to decline branch

When consentResponse.action === "accept" but consentResponse.uploadInfo is absent (a malformed Teams invoke), execution falls to the else branch, which logs "user declined file consent" and calls removeResolvedPendingUpload. This silently deletes the pending upload without ever completing the transfer, making a retry impossible without resending the file.

In practice Teams always supplies uploadInfo on accept, but a defensive split would make intent and error handling explicit:

if (consentResponse.action === "accept") {
  if (!consentResponse.uploadInfo) {
    log.debug?.("accept invoke missing uploadInfo, ignoring", { uploadId });
    return true; // leave pending upload intact for a potential retry
  }
  if (pendingFile) {
    // ... upload path
  } else {
    await context.sendActivity(expiredUploadMessage);
  }
} else {
  // User declined
  log.debug?.("user declined file consent", { uploadId });
  await removeResolvedPendingUpload(uploadId, pendingFile?.source, fsStore, log);
}

Prompt To Fix With AI

This is a comment left during a code review.
Path: extensions/msteams/src/monitor-handler.ts
Line: 264-314

Comment:
**Malformed accept falls through to decline branch**

When `consentResponse.action === "accept"` but `consentResponse.uploadInfo` is absent (a malformed Teams invoke), execution falls to the `else` branch, which logs "user declined file consent" and calls `removeResolvedPendingUpload`. This silently deletes the pending upload without ever completing the transfer, making a retry impossible without resending the file.

In practice Teams always supplies `uploadInfo` on accept, but a defensive split would make intent and error handling explicit:

```typescript
if (consentResponse.action === "accept") {
  if (!consentResponse.uploadInfo) {
    log.debug?.("accept invoke missing uploadInfo, ignoring", { uploadId });
    return true; // leave pending upload intact for a potential retry
  }
  if (pendingFile) {
    // ... upload path
  } else {
    await context.sendActivity(expiredUploadMessage);
  }
} else {
  // User declined
  log.debug?.("user declined file consent", { uploadId });
  await removeResolvedPendingUpload(uploadId, pendingFile?.source, fsStore, log);
}
```

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

Prompt To Fix All With AI

This is a comment left during a code review.
Path: extensions/msteams/src/monitor-handler.ts
Line: 264-314

Comment:
**Malformed accept falls through to decline branch**

When `consentResponse.action === "accept"` but `consentResponse.uploadInfo` is absent (a malformed Teams invoke), execution falls to the `else` branch, which logs "user declined file consent" and calls `removeResolvedPendingUpload`. This silently deletes the pending upload without ever completing the transfer, making a retry impossible without resending the file.

In practice Teams always supplies `uploadInfo` on accept, but a defensive split would make intent and error handling explicit:

```typescript
if (consentResponse.action === "accept") {
  if (!consentResponse.uploadInfo) {
    log.debug?.("accept invoke missing uploadInfo, ignoring", { uploadId });
    return true; // leave pending upload intact for a potential retry
  }
  if (pendingFile) {
    // ... upload path
  } else {
    await context.sendActivity(expiredUploadMessage);
  }
} else {
  // User declined
  log.debug?.("user declined file consent", { uploadId });
  await removeResolvedPendingUpload(uploadId, pendingFile?.source, fsStore, log);
}
```

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/msteams/src/pending-uploads-fs.ts
Line: 88-103

Comment:
**Index keys not validated before blob unlink in pruneExpired**

`pruneExpired` collects `id` values directly from `Object.entries(index.entries)` and passes them to `unlinkBlobIfPresent` without running `isSafeUploadId`. A tampered `index.json` with a path-traversal key would cause `blobPath(storeDir, id)` to resolve outside the store directory. `isSafeUploadId` already exists for exactly this purpose — consider applying it to pruned keys as well:

```typescript
for (const [id, entry] of Object.entries(index.entries)) {
  if (!isSafeUploadId(id)) {
    expired.push(id); // drop invalid keys from the index but skip blob unlink
    continue;
  }
  if (nowMs - entry.createdAt > ttlMs) {
    expired.push(id);
    continue;
  }
  kept[id] = entry;
}
```

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

_{Reviews (1): Last reviewed commit: "fix(msteams): handle fileConsent/invoke ..." | Re-trigger Greptile}

greptile-apps · 2026-04-10T04:57:07Z

+function pruneExpired(
+  index: PendingUploadFsIndex,
+  nowMs: number,
+  ttlMs: number,
+): { index: PendingUploadFsIndex; expiredIds: string[] } {
+  const kept: Record<string, PendingUploadFsEntry> = {};
+  const expired: string[] = [];
+  for (const [id, entry] of Object.entries(index.entries)) {
+    if (nowMs - entry.createdAt > ttlMs) {
+      expired.push(id);
+      continue;
+    }
+    kept[id] = entry;
+  }
+  return { index: { version: 1, entries: kept }, expiredIds: expired };
+}


Index keys not validated before blob unlink in pruneExpired

pruneExpired collects id values directly from Object.entries(index.entries) and passes them to unlinkBlobIfPresent without running isSafeUploadId. A tampered index.json with a path-traversal key would cause blobPath(storeDir, id) to resolve outside the store directory. isSafeUploadId already exists for exactly this purpose — consider applying it to pruned keys as well:

for (const [id, entry] of Object.entries(index.entries)) { if (!isSafeUploadId(id)) { expired.push(id); // drop invalid keys from the index but skip blob unlink continue; } if (nowMs - entry.createdAt > ttlMs) { expired.push(id); continue; } kept[id] = entry; }

Prompt To Fix With AI

This is a comment left during a code review. Path: extensions/msteams/src/pending-uploads-fs.ts Line: 88-103 Comment: **Index keys not validated before blob unlink in pruneExpired** `pruneExpired` collects `id` values directly from `Object.entries(index.entries)` and passes them to `unlinkBlobIfPresent` without running `isSafeUploadId`. A tampered `index.json` with a path-traversal key would cause `blobPath(storeDir, id)` to resolve outside the store directory. `isSafeUploadId` already exists for exactly this purpose — consider applying it to pruned keys as well: ```typescript for (const [id, entry] of Object.entries(index.entries)) { if (!isSafeUploadId(id)) { expired.push(id); // drop invalid keys from the index but skip blob unlink continue; } if (nowMs - entry.createdAt > ttlMs) { expired.push(id); continue; } kept[id] = entry; } ``` 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: 3f34fb7255

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-04-10T04:58:42Z

+      // Write the blob FIRST (best-effort crash safety: if we crash between
+      // blob write and index update, a stale blob without an index entry is
+      // just disk litter that the next prune cycle can skip).
+      await fs.promises.writeFile(blobPath(storeDir, id), new Uint8Array(params.buffer));


Write pending-upload blobs with 0600 permissions

The new filesystem store writes raw attachment bytes with fs.promises.writeFile(...) defaults, which creates files using process umask (commonly world/group-readable like 0644), and mkdir(..., { recursive: true }) also defaults to permissive directory modes. In deployments where OPENCLAW_STATE_DIR (or its parents) is shared/traversable by other users, pending uploads can be read before consent is handled, exposing sensitive file contents; this path should use explicit restrictive modes for both directory and blob files.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-04-10T04:58:42Z

+      const { activity, uploadId } = await prepareFileConsentActivityFs({
        media: { buffer: media.buffer, filename: fileName, contentType: media.contentType },
        conversationId,
        description: messageText || undefined,


Fallback to in-memory consent store when FS write fails

This path now always persists pending uploads to disk before sending the consent card, but there is no fallback if that write fails (for example read-only/unavailable state dir), so sendMessageMSTeams aborts and sends nothing. That is a regression for in-process sends (such as outbound/action flows in the running gateway) where the existing in-memory pending-upload store would still allow consent handling to work even without filesystem persistence.

Useful? React with 👍 / 👎.

- Adds consentCardActivityId to PendingUpload so the consent card activity can be replaced in-place after upload succeeds - Uses context.updateActivity() to replace the FileConsentCard with the file info card; falls back to sendActivity if update fails - Adds updateActivity to MSTeamsTurnContext type - Fixes timer leak in pending-uploads: clears TTL setTimeout on explicit removal and on clearPendingUploads() - Adds pending-uploads.test.ts covering all new timer/cleanup paths

…th updateActivity test Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cbed48138e

ℹ️ 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-04-10T15:40:32Z

+    const activity = await buildActivity(
+      message,
+      params.conversationRef,
+      params.tokenProvider,
+      params.sharePointSiteId,


Keep buildActivity inside retry loop

sendMessageInContext now builds the activity before entering sendWithRetry, so retries only wrap ctx.sendActivity. buildActivity includes non-trivial prep steps (including OneDrive/SharePoint upload paths) that can fail transiently, and those failures now bypass the configured retry path entirely. In environments where media upload/auth calls intermittently return retryable failures, this regresses reliability by failing the whole send on the first error instead of retrying the full attempt.

Useful? React with 👍 / 👎.

…ad-consent-55386

… upload (openclaw#55386) (openclaw#64087) * fix(msteams): update FileConsentCard after user accepts upload - Adds consentCardActivityId to PendingUpload so the consent card activity can be replaced in-place after upload succeeds - Uses context.updateActivity() to replace the FileConsentCard with the file info card; falls back to sendActivity if update fails - Adds updateActivity to MSTeamsTurnContext type - Fixes timer leak in pending-uploads: clears TTL setTimeout on explicit removal and on clearPendingUploads() - Adds pending-uploads.test.ts covering all new timer/cleanup paths * msteams: wire consentCardActivityId from send response + add happy-path updateActivity test Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(msteams): retry consent uploads end-to-end --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Brad Groux <3053586+BradGroux@users.noreply.github.com>

openclaw-barnacle Bot added channel: msteams Channel integration: msteams size: L labels Apr 10, 2026

greptile-apps Bot reviewed Apr 10, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Apr 10, 2026

View reviewed changes

BradGroux mentioned this pull request Apr 10, 2026

WORKING: All Microsoft Issues and PRs #49126

Closed

sudie-codes and others added 2 commits April 10, 2026 10:33

msteams: wire consentCardActivityId from send response + add happy-pa…

cbed481

…th updateActivity test Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

BradGroux force-pushed the fix/msteams-file-upload-consent-55386 branch from 3f34fb7 to cbed481 Compare April 10, 2026 15:33

openclaw-barnacle Bot added size: M and removed size: L labels Apr 10, 2026

chatgpt-codex-connector Bot reviewed Apr 10, 2026

View reviewed changes

BradGroux added 2 commits April 10, 2026 14:16

fix(msteams): retry consent uploads end-to-end

03b0c22

Merge remote-tracking branch 'origin/main' into fix/msteams-file-uplo…

61b7970

…ad-consent-55386

BradGroux merged commit 7843187 into openclaw:main Apr 10, 2026
7 checks passed

sudie-codes mentioned this pull request Apr 10, 2026

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

Closed

3 tasks

ndholakia mentioned this pull request Apr 10, 2026

fix(msteams): forward messageBack card actions (Action.Submit) to agent (#60952) #64503

Open

5 tasks

BingqingLyu mentioned this pull request Apr 28, 2026

fix(msteams): forward messageBack card actions (Action.Submit) to agent (#60952) BingqingLyu/openclaw#2279

Open

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(msteams): handle fileConsent/invoke callback for bot-to-user file upload (#55386)#64087

fix(msteams): handle fileConsent/invoke callback for bot-to-user file upload (#55386)#64087
BradGroux merged 4 commits intoopenclaw:mainfrom
sudie-codes:fix/msteams-file-upload-consent-55386

sudie-codes commented Apr 10, 2026

Uh oh!

greptile-apps Bot commented Apr 10, 2026 •

edited

Loading

Comments Outside Diff (1)

Uh oh!

greptile-apps Bot Apr 10, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Apr 10, 2026

Uh oh!

chatgpt-codex-connector Bot Apr 10, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Apr 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

sudie-codes commented Apr 10, 2026

Summary

Fix

Test plan

Uh oh!

greptile-apps Bot commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Comments Outside Diff (1)

Uh oh!

greptile-apps Bot Apr 10, 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 Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Apr 10, 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 Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

greptile-apps Bot commented Apr 10, 2026 •

edited

Loading