fix(msteams): cap team-group-id cache with pruneMapToMaxSize by sunlit-deng · Pull Request #101964 · openclaw/openclaw

sunlit-deng · 2026-07-08T02:21:16Z

What Problem This Solves

The teamGroupIdCache Map in resolveTeamGroupId had TTL-based expiry (10 min) but no size cap. A large Teams workspace with many distinct team IDs could accumulate entries faster than the TTL window, allowing unbounded growth.

Why This Change Was Made

Added a 500-entry cap via pruneMapToMaxSize — the shared SDK helper already used by 11 other files. The cap is applied after each successful cache insert.

User Impact

No change for normal usage. Memory usage bounded at ~500 entries regardless of workspace size.

Evidence

Real-path proof — exercises the full resolveTeamGroupId → fetch → fetchGraphJson → .set() → pruneMapToMaxSize production pipeline with tsx (mocks only globalThis.fetch, everything else is production code):

$ npx tsx proof-live.ts
=== live-proof: resolveTeamGroupId production insert/prune path ===

1. 500th insert via resolveTeamGroupId: cache.size = 500
2.   resolveTeamGroupId("team-499") → groupId = guid-team-499
3. 501st insert via resolveTeamGroupId: cache.size = 500
4.   resolveTeamGroupId("team-500") → groupId = guid-team-500
5.   team-0 evicted: true
6. After 1000 resolveTeamGroupId calls: cache.size = 500
7.   team-999 cached: true
8.   Cache hit team-999: groupId = guid-team-999 (no fetch)

=== PROOF COMPLETE ===

499 entries pre-filled → 500th resolveTeamGroupId goes through production fetch → fetchGraphJson → set path
501st call: hits the production pruneMapToMaxSize line at graph-thread.ts:89 → oldest evicted
1000 total calls through resolveTeamGroupId: cache stays at 500
Cache-hit returns correct groupId without Graph fetch

proof-live.ts

import { _teamGroupIdCacheForTest, resolveTeamGroupId } from "./extensions/msteams/src/graph-thread";

const CAP = 500;

console.log("=== live-proof: resolveTeamGroupId production insert/prune path ===\n");

// Mock global fetch so resolveTeamGroupId's internal Graph call succeeds.
// The fetch → requestGraph → fetchGraphJson → resolveTeamGroupId pipeline
// exercises the real production insert+prune code at graph-thread.ts:89.
const origFetch = globalThis.fetch as typeof globalThis.fetch;
globalThis.fetch = (async (url: string | URL, _init?: RequestInit) => {
  const urlStr = String(url);
  const teamId = urlStr.split("/teams/")[1]?.split("?")[0] || "unknown";
  return new Response(JSON.stringify({ id: `guid-${teamId}` }), {
    status: 200,
    headers: { "Content-Type": "application/json" },
  });
}) as typeof globalThis.fetch;

// Pre-fill 499 entries directly (avoids 499 wasteful fetch calls).
// resolveTeamGroupId cache-hit path skips Graph, so these are harmless.
for (let i = 0; i < CAP - 1; i++) {
  _teamGroupIdCacheForTest.set(`team-${i}`, {
    groupId: `guid-team-${i}`,
    expiresAt: Date.now() + 600_000,
  });
}

// 500th call: cache miss → fetchGraphJson → set + prune (full production path)
const r499 = await resolveTeamGroupId("token", "team-499");
console.log(`1. 500th insert via resolveTeamGroupId: cache.size = ${_teamGroupIdCacheForTest.size}`);
console.log(`2.   resolveTeamGroupId("team-499") → groupId = ${r499}`);

// 501st call triggers prune internally — oldest entry (team-0) evicted
const r500 = await resolveTeamGroupId("token", "team-500");
console.log(`3. 501st insert via resolveTeamGroupId: cache.size = ${_teamGroupIdCacheForTest.size}`);
console.log(`4.   resolveTeamGroupId("team-500") → groupId = ${r500}`);
console.log(`5.   team-0 evicted: ${!_teamGroupIdCacheForTest.has("team-0")}`);

// Fill 500 more entries via resolveTeamGroupId — size stays at CAP
for (let i = 501; i < 1000; i++) {
  await resolveTeamGroupId("token", `team-${i}`);
}
console.log(`6. After 1000 resolveTeamGroupId calls: cache.size = ${_teamGroupIdCacheForTest.size}`);
console.log(`7.   team-999 cached: ${_teamGroupIdCacheForTest.has("team-999")}`);

// Cache hit still works — no Graph API call for cached entries
const hit = await resolveTeamGroupId("token", "team-999");
console.log(`8.   Cache hit team-999: groupId = ${hit} (no fetch)`);

console.log(`\n=== PROOF COMPLETE ===`);

Vitest — 27/27 passed including cache cap regression:

$ vitest run --project extension-msteams extensions/msteams/src/graph-thread.test.ts

 ✓ resolveTeamGroupId > caps cache at 500 entries — evicts oldest on overflow
 ... 27 passed (27)

AI-assisted: built with Codex

clawsweeper · 2026-07-08T02:27:57Z

Codex review: needs maintainer review before merge. Reviewed July 8, 2026, 8:15 AM ET / 12:15 UTC.

Summary
This PR imports the plugin SDK bounded-map helper, caps the MS Teams resolveTeamGroupId cache at 500 entries after successful inserts, and adds a regression test for overflow eviction.

PR surface: Source +3, Tests +24. Total +27 across 2 files.

Reproducibility: yes. Source inspection shows current main stores distinct successful Teams Graph lookups in a TTL-only Map, and the PR test/proof exercises the 501-entry overflow path.

Review metrics: none identified.

Stored data model
Persistent data-model change detected: persistent cache schema: extensions/msteams/src/graph-thread.test.ts, vector/embedding metadata: extensions/msteams/src/graph-thread.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🦞 diamond lobster
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:

[P2] Refresh or rebase against current main and let exact-head checks rerun before merge.

Risk before merge

[P1] The branch is behind current main, so maintainers should refresh the branch and rerun exact-head checks before landing.

Maintainer options:

Decide the mitigation before merge
Land the focused cache cap after a branch refresh confirms exact-head CI and mergeability.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

[P2] No repair lane is needed because there are no blocking findings; maintainers should review, refresh the branch, and merge if exact-head checks pass.

Security
Cleared: The diff only adds bounded in-memory pruning through an existing SDK helper and a colocated test, with no dependency, workflow, secret, package, or code-execution surface change.

Review details

Best possible solution:

Land the focused cache cap after a branch refresh confirms exact-head CI and mergeability.

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

Yes. Source inspection shows current main stores distinct successful Teams Graph lookups in a TTL-only Map, and the PR test/proof exercises the 501-entry overflow path.

Is this the best way to solve the issue?

Yes. Pruning immediately after successful cache insertion with the existing SDK helper is the narrow maintainable fix and avoids duplicating local LRU code or adding configuration.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 4bf70be01a21.

Label changes

Label changes:

add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes terminal output from a tsx proof that drives resolveTeamGroupId through the production fetch/set/prune path with mocked Graph responses and shows cap, eviction, and cache-hit behavior.
add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes terminal output from a tsx proof that drives resolveTeamGroupId through the production fetch/set/prune path with mocked Graph responses and shows cap, eviction, and cache-hit behavior.
remove rating: 🦪 silver shellfish: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

P2: This is a bounded Microsoft Teams memory-risk fix in the optional thread-context path with limited blast radius.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes terminal output from a tsx proof that drives resolveTeamGroupId through the production fetch/set/prune path with mocked Graph responses and shows cap, eviction, and cache-hit behavior.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes terminal output from a tsx proof that drives resolveTeamGroupId through the production fetch/set/prune path with mocked Graph responses and shows cap, eviction, and cache-hit behavior.

Evidence reviewed

PR surface:

Source +3, Tests +24. Total +27 across 2 files.

View PR surface stats

Area	Files	Added	Net
Source	1	3	+3
Tests	1	24	+24
Docs	0	0	0
Config	0	0	0
Generated	0	0	0
Other	0	0	0
Total	2	27	+27

What I checked:

Current main behavior: Current main caches every successful resolveTeamGroupId lookup in a module-level TTL Map and has no size cap after insertion. (extensions/msteams/src/graph-thread.ts:83, 4bf70be01a21)
PR runtime change: The PR head imports pruneMapToMaxSize, defines a 500-entry cap, and prunes immediately after a successful cache insert. (extensions/msteams/src/graph-thread.ts:89, b28923cce044)
Regression coverage: The added test fills 500 unique team IDs, inserts a 501st, verifies the oldest key is fetched again, and verifies the newest key remains cached. (extensions/msteams/src/graph-thread.test.ts:126, b28923cce044)
Helper contract: pruneMapToMaxSize prunes insertion-ordered oldest Map keys until the map fits the requested limit, matching the PR's intended eviction behavior. (src/infra/map-size.ts:13, 4bf70be01a21)
Plugin SDK boundary: openclaw/plugin-sdk/collection-runtime is an exported SDK subpath, so the extension import follows the scoped plugin boundary. (package.json:787, 4bf70be01a21)
Runtime caller: Teams channel thread handling resolves the group ID through resolveTeamGroupId before fetching parent and reply context, so the cache sits on the real inbound thread-context path. (extensions/msteams/src/monitor-handler/message-handler.ts:648, 4bf70be01a21)

Likely related people:

sudie-codes: Merged PR history shows this account authored the Teams thread-history feature that introduced graph-thread.ts, resolveTeamGroupId, the TTL-only team-group cache, and adjacent tests. (role: feature originator; confidence: high; commits: 8c852d86f759; files: extensions/msteams/src/graph-thread.ts, extensions/msteams/src/graph-thread.test.ts, extensions/msteams/src/monitor-handler/message-handler.ts)
vincentkoc: Current-main blame and PR history show this account carried the Teams graph-thread code through the recent plugin refactor. (role: recent area contributor; confidence: medium; commits: cc083e5f4fe0, e085fa1a3ffd; files: extensions/msteams/src/graph-thread.ts, extensions/msteams/src/graph-thread.test.ts)
jacobtomlinson: Merged history shows adjacent Teams thread-history allowlist work in the same inbound thread-context path. (role: adjacent contributor; confidence: medium; commits: 5cca38084074; files: extensions/msteams/src/monitor-handler/message-handler.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 (13 earlier review cycles; latest 8 shown)

reviewed 2026-07-08T04:14:48.198Z sha 0b00562 :: needs real behavior proof before merge. :: [P2] Reset the cache before the cap regression
reviewed 2026-07-08T05:32:57.117Z sha c695b20 :: needs real behavior proof before merge. :: [P1] Restore the SDK import as a top-level import | [P1] Move pruning after the cache insert | [P2] Reset the cache before the cap regression
reviewed 2026-07-08T06:43:57.533Z sha ac886fd :: needs real behavior proof before merge. :: none
reviewed 2026-07-08T07:18:39.975Z sha 1304d3c :: needs real behavior proof before merge. :: none
reviewed 2026-07-08T07:39:23.824Z sha 98f54d9 :: needs real behavior proof before merge. :: [P3] Add braces to the raw Map loop
reviewed 2026-07-08T07:48:43.293Z sha b28923c :: needs real behavior proof before merge. :: none
reviewed 2026-07-08T09:59:03.533Z sha b28923c :: needs real behavior proof before merge. :: none
reviewed 2026-07-08T12:08:42.803Z sha b28923c :: needs maintainer review before merge. :: none

sunlit-deng · 2026-07-08T02:44:19Z

@clawsweeper re-review

clawsweeper · 2026-07-08T02:44:21Z

🦞🧹
ClawSweeper re-review requested.