Greptile Summary

This PR re-lands the revoked-context proactive fallback for MS Teams (#27224) that was lost in a subsequent force-push to main. It refactors the thread-send path in messenger.ts into two reusable helpers (sendMessagesInContext / sendProactively) and wraps each per-message send in a try/catch that detects a revoked-proxy TypeError and falls through to proactive delivery for that message and all remaining ones. A new isRevokedProxyError() utility is added to errors.ts and two targeted integration tests are added.

Key observations:

• Core logic is correct: when a revoked proxy error is caught at index idx, messages.slice(idx) correctly includes the failing message so nothing is lost, and non-revocation errors still propagate normally.
• The retry loop in sendWithRetry will not inadvertently retry a revoked-proxy TypeError because classifyMSTeamsSendError classifies it as "unknown" and shouldRetry returns false for that kind — the error surfaces immediately to the outer handler as expected.
• The proactive fallback strips activityId from the conversation reference, sending fallback messages to the conversation root rather than as thread replies, which is the best available behavior when the thread context is gone.
• Minor: the if (remaining.length > 0) guard (line 491) is always true by loop invariant and is effectively dead code.
• isRevokedProxyError has no dedicated unit tests in errors.test.ts; it is only exercised through the new integration tests.

Confidence Score: 4/5

Safe to merge — the re-landed logic is correct and well-tested; only minor style nits remain.

The core fallback logic is sound: per-message try/catch, correct slice semantics (failing message is included in the proactive batch), non-revocation errors still propagate, and retry does not fire for revoked-proxy TypeErrors. Two new integration tests cover both full and partial revocation. Small deductions for the always-true remaining.length > 0 guard (dead code) and the absence of dedicated unit tests for isRevokedProxyError in errors.test.ts.

No files require special attention; the two style items are non-blocking.

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

Comment:
**Redundant guard — `remaining` is always non-empty here**

`messages.slice(idx)` always includes at least `messages[idx]` (the currently-failing message), so `remaining.length > 0` is always `true` at this point. The `if` branch is dead code and can be removed to avoid confusion for future readers.

```suggestion
        messageIds.push(...(await sendProactively(remaining, idx)));
```

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/errors.ts
Line: 177-182

Comment:
**No dedicated unit tests for `isRevokedProxyError`**

`isRevokedProxyError` is only exercised indirectly through the `messenger.test.ts` integration tests. Consider adding a small unit test block in `errors.test.ts` covering:
- non-`TypeError` values (plain `Error`, a string, `null`) → should return `false`
- a `TypeError` with an unrelated message → should return `false`
- a `TypeError` with the canonical revoked-proxy message → should return `true`

This makes the utility's contract explicit and guards against future regressions to the regex independently of the messenger layer.

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

_{Reviews (1): Last reviewed commit: "fix(msteams): re-land revoked-context pr..." | Re-trigger Greptile}

Closing — the revoked-context fallback from #27224 was already re-landed upstream as part of 866bd91 (refactor: harden msteams lifecycle and attachment flows), which extracted withRevokedProxyFallback into revoked-context.ts. The functionality is already on main.

Our fork was behind upstream/main, which made it look missing locally. No action needed.