fix(msteams): bound probe token acquisition to request deadline by wangmiao0668000666 · Pull Request #106386 · openclaw/openclaw

wangmiao0668000666 · 2026-07-13T12:38:25Z

What Problem This Solves

MS Teams token acquisition could wait forever when the Teams SDK or its authentication transport stalled. The unbounded calls affected both probeMSTeams() phases and the shared resolveGraphToken() path used by Teams Graph actions.

Why This Change Was Made

All three missing calls now reuse the plugin's existing withMSTeamsRequestDeadline helper and its established 30-second default:

Bot Framework probe token
Graph probe token
shared Graph action token

This is the existing owner-local deadline contract already used by Teams attachment, media, identity, and message paths. No new helper, config, environment variable, SDK surface, fallback, or capability is introduced.

The regression coverage drives the real provider functions with never-settling SDK promises under fake timers. The original standalone 168-line test was rewritten into two focused suites covering the probe and shared Graph resolver, reducing the final PR by 13 lines while adding the missed Graph production path.

User Impact

Teams health probes and Graph-backed actions now return a labeled timeout error instead of remaining pending indefinitely when token acquisition stalls. Existing successful, credential-error, delegated-token, and China-cloud behavior is unchanged.

Evidence

Dependency contract:

@microsoft/teams.apps 2.0.13 TokenManager.getBotToken() and getGraphToken() call MSAL or managed-identity token acquisition without accepting an abort signal or per-call timeout.
The existing OpenClaw Teams request deadline is therefore the correct owner boundary for bounding these non-abortable SDK promises.

Focused tests after the final rewrite:

node scripts/run-vitest.mjs extensions/msteams/src/probe.test.ts extensions/msteams/src/probe.timeout.test.ts extensions/msteams/src/graph.test.ts extensions/msteams/src/graph.timeout.test.ts extensions/msteams/src/request-timeout.test.ts

Result: 5 files passed, 33 tests passed.

Hosted changed gate:

node scripts/check-changed.mjs -- extensions/msteams/src/graph.ts extensions/msteams/src/graph.timeout.test.ts extensions/msteams/src/probe.ts extensions/msteams/src/probe.timeout.test.ts

Result: passed on Blacksmith Testbox tbx_01kxs8nxvngn01fktwyjdmgdjj, including formatting, extension production/test typechecks, lint, Plugin SDK manifest, plugin boundaries, repository guards, and import-cycle checks: https://github.com/openclaw/openclaw/actions/runs/29622311925

Fresh autoreview: no findings; patch correct at 0.93 confidence.

The contributor's controlled transport proof also exercised the production Teams SDK and real @azure/msal-node path against a loopback response-body stall. The probe returned at 30,002 ms with the expected timeout and the following Graph acquisition remained usable. No credentials or live Azure traffic were used.

Changelog

No CHANGELOG.md edit in this contributor PR. Release-note context is preserved here and in the squash message.

clawsweeper · 2026-07-13T13:38:54Z

Codex review: needs maintainer review before merge. Reviewed July 17, 2026, 8:14 PM ET / July 18, 2026, 00:14 UTC.

Summary
The branch wraps MS Teams probe and shared Graph token acquisition in the existing request-deadline helper and adds fake-timer timeout regression coverage.

PR surface: Source +13, Tests +154. Total +167 across 4 files.

Reproducibility: yes. source inspection gives a high-confidence path: make the Teams SDK token-provider promise never settle and call the probe or shared Graph resolver; the new focused tests exercise that path with fake timers.

Review metrics: none identified.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

Wait for the current exact-head required checks, then obtain the assigned maintainer's landing decision.

Next step before merge

[P2] No mechanical repair is indicated; the active assigned maintainer should review exact-head check completion and decide whether to land the PR.

Security
Cleared: The four-file TypeScript/test diff adds no dependency, credential, permission, package-resolution, workflow, or supply-chain surface.

Review details

Best possible solution:

Land the exact reviewed head once required checks complete, retaining the existing 30-second MS Teams deadline contract for stalled SDK token acquisition.

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

Yes, source inspection gives a high-confidence path: make the Teams SDK token-provider promise never settle and call the probe or shared Graph resolver; the new focused tests exercise that path with fake timers.

Is this the best way to solve the issue?

Yes. Reusing the existing owner-local request-deadline helper at each previously unbounded token-acquisition await is narrower and safer than adding a new setting, SDK wrapper, or fallback.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 72e4b779215b.

Label changes

Label justifications:

P2: The patch prevents indefinite MS Teams probe and Graph-action waits, but its failure mode is limited to a channel plugin and has no evidence of broader runtime impact.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The supplied exact-head review context reports a controlled production Teams SDK and MSAL transport-stall run that reached the 30-second timeout and then completed a subsequent Graph acquisition without using credentials or live Azure traffic.
proof: sufficient: Contributor real behavior proof is sufficient. The supplied exact-head review context reports a controlled production Teams SDK and MSAL transport-stall run that reached the 30-second timeout and then completed a subsequent Graph acquisition without using credentials or live Azure traffic.

Evidence reviewed

PR surface:

Source +13, Tests +154. Total +167 across 4 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	2	16	3	+13
Tests	2	154	0	+154
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	4	170	3	+167

What I checked:

Current-head probe implementation: The PR patch wraps both probeMSTeams() token-provider calls with the existing withMSTeamsRequestDeadline helper using distinct Bot Framework and Graph labels, preserving the surrounding result/error handling. (extensions/msteams/src/probe.ts:76, 39d98942d8f5)
Shared Graph path covered: The branch also wraps resolveGraphToken() token acquisition, covering the shared Graph action path rather than fixing only the health probe. (extensions/msteams/src/graph.ts:247, 39d98942d8f5)
Regression coverage: The added timeout suites drive never-settling provider promises under fake timers and assert the established 30,000 ms request deadline, avoiding production-length waits in unit tests. (extensions/msteams/src/probe.timeout.test.ts:46, 39d98942d8f5)
Feature-history and active ownership: The current PR head includes follow-up commits ad6fc3385a10116292433c92b2dd73da97fcab1d and 39d98942d8f50e98a6579f60d821151009bb8f2f by steipete, and the timeline records assignment on 2026-07-17; this is active maintainer adoption rather than an abandoned contributor branch. (extensions/msteams/src/graph.ts:247, 39d98942d8f5)
Exact-head proof and validation context: The PR reports a controlled real Teams SDK and MSAL transport-stall run that timed out at the production deadline and recovered for a subsequent Graph acquisition; the supplied review context marks proof sufficient and reports focused tests plus a hosted changed-path gate. (extensions/msteams/src/graph.timeout.test.ts:41, 39d98942d8f5)

Likely related people:

steipete: Assigned this PR on 2026-07-17 and authored the two latest follow-up commits that complete the remaining token-acquisition and fixture work. (role: assigned reviewer and current-head follow-up author; confidence: high; commits: ad6fc3385a10, 39d98942d8f5; files: extensions/msteams/src/graph.ts, extensions/msteams/src/probe.timeout.test.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

Review history (3 earlier review cycles)

reviewed 2026-07-13T13:36:48.508Z sha 082f48e :: needs real behavior proof before merge. :: [P2] Avoid waiting through production deadlines in unit tests
reviewed 2026-07-13T18:07:51.593Z sha 512ab8c :: needs maintainer review before merge. :: none
reviewed 2026-07-17T15:01:44.520Z sha 5da5ee6 :: needs maintainer review before merge. :: none

wangmiao0668000666 · 2026-07-13T16:03:51Z

@clawsweeper re-review

Updated PR to address both blockers from the prior 🦪 review:

P2 (automation / 🚨 merge-risk): rewrote extensions/msteams/src/probe.timeout.test.ts to drive withTimeout's internal setTimeout via vi.useFakeTimers() + vi.advanceTimersByTimeAsync(...) instead of waiting 30 wall-clock seconds per case. Per-case test wall-clock is now 25ms / 3ms / 2ms / 1ms / 2ms (was 30s / 30s / 30s / 2ms / n/a). One new case asserts the production default deadline is exactly MSTEAMS_REQUEST_TIMEOUT_MS = 30_000 by spying on withTimeout's timeoutMs argument, so the production contract is not silently weakened.
P1 (needs proof): added section 4.2 to the PR body — an exact-head controlled local MS Teams SDK transport proof. A temporary uncommitted scratchpad/msteams-probe-real-sdk-proof.mts harness drove the production probeMSTeams entry point through the real @microsoft/teams.apps App + real @azure/msal-node ConfidentialClientApplication.acquireTokenByClientCredential, with MSAL's outbound fetch redirected (via a globalThis.fetch monkey-patch) to a local node:http server that selectively stalled the body. Transcript shows the bot call aborting at the production 30-second deadline (probe: bot elapsed=30002ms) and the graph call completing cleanly in 17ms on the recovery phase, proving the wrapper does not leak state across calls. Harness file was deleted after the run.

Both new commits are on fix/msteams-probe-token-deadline (HEAD 512ab8c926); CI has not yet re-run on the new commits but the change is test-only (no source change).

clawsweeper · 2026-07-13T16:03:55Z

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

Re-review progress:

State: Complete
Detail: The targeted re-review published an exact durable review comment and queued an exact-item serialized router run.
Run: https://github.com/openclaw/clawsweeper/actions/runs/29272807174
Updated: 2026-07-13T18:10:11.523Z

wangmiao0668000666 · 2026-07-17T14:55:25Z

Rebased onto current main (patch-identical cherry-pick, verified via git patch-id; diff unchanged: extensions/msteams/src/probe.ts + probe.timeout.test.ts).

The failing checks-node-compact-small-1 shard was unrelated to this diff: test/scripts/test-extension.test.ts ("counts tracked extension tests without walking extension directories") asserted zero readdirSync calls, but the branch's old base ran the pre-fix scripts/lib/extension-test-plan.mjs where unscoped git ls-files output can exceed spawnSync's 1 MiB default buffer and fall back to directory walks. Main fixed this in 242cdca (scoped ls-files -- <pathspecs> + explicit buffer, "Large checkouts exceed Node's 1 MiB spawnSync default") after this branch's base. The rebased head carries that fix, so the shard should pass now.

probeMSTeams() at extensions/msteams/src/probe.ts:75 and :89 awaited tokenProvider.getAccessToken(...) for the Bot Framework and Microsoft Graph token endpoints with no surrounding deadline. The Microsoft Teams SDK does not carry an inherent timeout on these calls, so a stalled Azure AD token endpoint pinned the probe indefinitely. Wrap both awaits with withMSTeamsRequestDeadline (default MSTEAMS_REQUEST_TIMEOUT_MS = 30_000), matching the pattern already used by six other MS Teams call sites: attachments/bot-framework.ts:252, attachments/graph.ts:258, monitor-handler/message-handler.ts:594/654/685/692, attachments/download.ts:167, team-identity.ts:37. The probe was the one missing site. No new helper, no SDK change. The existing outer catch at probe.ts:138 and inner catch at probe.ts:110 convert the timeout into a ProbeMSTeamsResult with ok: false and a structured error field. Added probe.timeout.test.ts: real probeMSTeams() with vi.mock injected never-resolving getBotToken/getGraphToken; asserts the call returns within the 30s bound instead of hanging to the proof budget.

The original probe.timeout.test.ts waited 90 seconds of wall-clock per focused run (3 stalled cases racing against a real setTimeout budget). Per ClawSweeper P2 (automation), this material deterministic CI cost can slow or time out test shards. Drive the withTimeout race (from @openclaw/fs-safe/dist/timing.js, uses setTimeout + clearTimeout) via vi.useFakeTimers() so each stalled case resolves in milliseconds. Add one new case that spies on withTimeout's timeoutMs argument to assert the production default deadline is exactly MSTEAMS_REQUEST_TIMEOUT_MS = 30_000, so the production contract is not silently weakened by the fake-timer change. Per-case wall-clock: 25ms / 3ms / 2ms / 1ms / 2ms (was: 30s / 30s / 30s / 2ms / n/a). Co-Authored-By: Claude <[email protected]>

steipete · 2026-07-18T00:21:54Z

Merged via squash.

Prepared head SHA: 39d98942d8f50e98a6579f60d821151009bb8f2f
Landed commit: dc0285366efcfd3130f75349af030fdde3a86dcb

…claw#106386) * fix(msteams): bound probe token acquisition to request deadline probeMSTeams() at extensions/msteams/src/probe.ts:75 and :89 awaited tokenProvider.getAccessToken(...) for the Bot Framework and Microsoft Graph token endpoints with no surrounding deadline. The Microsoft Teams SDK does not carry an inherent timeout on these calls, so a stalled Azure AD token endpoint pinned the probe indefinitely. Wrap both awaits with withMSTeamsRequestDeadline (default MSTEAMS_REQUEST_TIMEOUT_MS = 30_000), matching the pattern already used by six other MS Teams call sites: attachments/bot-framework.ts:252, attachments/graph.ts:258, monitor-handler/message-handler.ts:594/654/685/692, attachments/download.ts:167, team-identity.ts:37. The probe was the one missing site. No new helper, no SDK change. The existing outer catch at probe.ts:138 and inner catch at probe.ts:110 convert the timeout into a ProbeMSTeamsResult with ok: false and a structured error field. Added probe.timeout.test.ts: real probeMSTeams() with vi.mock injected never-resolving getBotToken/getGraphToken; asserts the call returns within the 30s bound instead of hanging to the proof budget. * test(msteams): drive probe timeout test with vi.useFakeTimers The original probe.timeout.test.ts waited 90 seconds of wall-clock per focused run (3 stalled cases racing against a real setTimeout budget). Per ClawSweeper P2 (automation), this material deterministic CI cost can slow or time out test shards. Drive the withTimeout race (from @openclaw/fs-safe/dist/timing.js, uses setTimeout + clearTimeout) via vi.useFakeTimers() so each stalled case resolves in milliseconds. Add one new case that spies on withTimeout's timeoutMs argument to assert the production default deadline is exactly MSTEAMS_REQUEST_TIMEOUT_MS = 30_000, so the production contract is not silently weakened by the fake-timer change. Per-case wall-clock: 25ms / 3ms / 2ms / 1ms / 2ms (was: 30s / 30s / 30s / 2ms / n/a). Co-Authored-By: Claude <[email protected]> * fix(msteams): bound remaining token acquisition * test(msteams): keep credential fixture unchanged --------- Co-authored-by: Claude <[email protected]> Co-authored-by: Peter Steinberger <[email protected]>

openclaw-barnacle Bot added channel: msteams Channel integration: msteams size: S labels Jul 13, 2026

wangmiao0668000666 force-pushed the fix/msteams-probe-token-deadline branch 2 times, most recently from a844c4f to 512ab8c Compare July 13, 2026 17:20

BradGroux mentioned this pull request Jul 14, 2026

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

Draft

wangmiao0668000666 force-pushed the fix/msteams-probe-token-deadline branch from 512ab8c to 5da5ee6 Compare July 17, 2026 14:55

steipete self-assigned this Jul 17, 2026

steipete force-pushed the fix/msteams-probe-token-deadline branch from 5da5ee6 to 4588249 Compare July 18, 2026 00:11

wangmiao0668000666 and others added 4 commits July 18, 2026 01:13

fix(msteams): bound remaining token acquisition

ad6fc33

test(msteams): keep credential fixture unchanged

39d9894

steipete force-pushed the fix/msteams-probe-token-deadline branch from 4588249 to 39d9894 Compare July 18, 2026 00:14

steipete merged commit dc02853 into openclaw:main Jul 18, 2026
85 checks passed

github-actions Bot mentioned this pull request Jul 18, 2026

📡 Upstream Digest — 2026-07-18 01:45 UTC curtismercier/openclaw-mods#1329

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(msteams): bound probe token acquisition to request deadline#106386

fix(msteams): bound probe token acquisition to request deadline#106386
steipete merged 4 commits into
openclaw:mainfrom
wangmiao0668000666:fix/msteams-probe-token-deadline

wangmiao0668000666 commented Jul 13, 2026 •

edited by steipete

Loading

Uh oh!

clawsweeper Bot commented Jul 13, 2026 •

edited

Loading

Uh oh!

wangmiao0668000666 commented Jul 13, 2026

Uh oh!

clawsweeper Bot commented Jul 13, 2026 •

edited

Loading

Uh oh!

wangmiao0668000666 commented Jul 17, 2026

Uh oh!

Uh oh!

steipete commented Jul 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

wangmiao0668000666 commented Jul 13, 2026 • edited by steipete Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Changelog

Uh oh!

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

Uh oh!

Uh oh!

wangmiao0668000666 commented Jul 13, 2026

Uh oh!

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

Uh oh!

Uh oh!

wangmiao0668000666 commented Jul 17, 2026

Uh oh!

Uh oh!

steipete commented Jul 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

wangmiao0668000666 commented Jul 13, 2026 •

edited by steipete

Loading

clawsweeper Bot commented Jul 13, 2026 •

edited

Loading

clawsweeper Bot commented Jul 13, 2026 •

edited

Loading