fix(slack): conversation lookups no longer grow cache without bound by zhangguiping-xydt · Pull Request #101562 · openclaw/openclaw

zhangguiping-xydt · 2026-07-07T10:20:04Z

What Problem This Solves

Fixes an issue where long-running Slack channel workers could keep every distinct conversation-info lookup in process memory indefinitely when many workspaces or channel IDs are resolved over time.

No linked issue. I checked the latest origin/main before the change and the Slack conversation-info cache was still an unbounded Map. I also searched open PRs for the exact cache symbol, the Slack channel-type path, slack cache, and slack conversation; I did not find an open PR handling this cache growth.

Why This Change Was Made

The Slack conversation-info cache now has a 1024-entry LRU limit. Cache hits are promoted, every cache write goes through the bounded helper, and the oldest non-promoted entry is evicted before inserting a new distinct account/channel key beyond the limit.

This does not change Slack config, schema, persisted state, Slack API request shape, or channel classification rules; it only bounds the process-local metadata cache.

User Impact

Long-running Slack deployments can continue resolving many channels without unbounded process-local memory growth from conversation metadata. Recently used conversation info remains cached, while stale entries are eligible for recomputation.

Evidence

RED regression proof before the fix:

FAIL  extensions/slack/src/channel-type.test.ts > resolveSlackChannelType > evicts least-recently-used conversation info entries after the cache limit
AssertionError: expected "vi.fn()" to be called 1026 times, but got 1025 times

Focused regression test after the fix:

$ node scripts/run-vitest.mjs run extensions/slack/src/channel-type.test.ts
Test Files  1 passed (1)
Tests  7 passed (7)

Issue-path runtime proof using the real Slack conversation resolver without Slack client mocks:

$ node --import tsx slack-cache-runtime-proof.mjs
Slack conversation info cache LRU runtime proof passed
warmed_entries=1024 inserted_extra_channel=1 evicted_channel=C00000001 promoted_channel=C00000000

Static checks:

$ pnpm tsgo:extensions:test
result: pass

$ pnpm lint:extensions
result: pass

OpenClaw gateway startup smoke after the patch:

=== GATEWAY READY
http server listening
ready

clawsweeper · 2026-07-07T10:32:53Z

Codex review: needs maintainer review before merge. Reviewed July 7, 2026, 6:37 AM ET / 10:37 UTC.

Summary
The PR adds a 1024-entry LRU bound to Slack conversation-info caching and adds a regression test for cache promotion and eviction.

PR surface: Source +25, Tests +49. Total +74 across 2 files.

Reproducibility: yes. Source inspection of current main shows every resolved Slack account/channel key is retained in an unbounded module Map; the PR body also includes a red regression test for eviction before the fix.

Review metrics: none identified.

Stored data model
Persistent data-model change detected: persistent cache schema: extensions/slack/src/channel-type.test.ts, persistent cache schema: extensions/slack/src/channel-type.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
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:

none.

Next step before merge

No ClawSweeper repair job is needed; this PR has no concrete review finding and should proceed through normal maintainer review and merge gates.

Security
Cleared: Cleared: the diff only changes Slack plugin TypeScript and tests, with no dependency, workflow, secret, permission, package, or code-execution surface changes.

Review details

Best possible solution:

Land a bounded process-local Slack conversation-info cache that preserves resolver semantics and recomputes evicted entries through the existing Slack-owned lookup path.

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

Yes. Source inspection of current main shows every resolved Slack account/channel key is retained in an unbounded module Map; the PR body also includes a red regression test for eviction before the fix.

Is this the best way to solve the issue?

Yes. The fix stays inside the Slack-owned helper, follows existing bounded Map cache patterns, preserves config/API/persisted-state behavior, and adds focused regression coverage for the new invariant.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 942b44966107.

Label changes

Label changes:

add P2: The PR fixes bounded memory growth in a Slack channel worker path with limited blast radius and no evidence of an urgent outage.
add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied terminal output for an after-fix runtime cache proof plus focused regression/static checks, which is enough for this non-visual process-cache behavior.
add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes copied terminal output for an after-fix runtime cache proof plus focused regression/static checks, which is enough for this non-visual process-cache behavior.

Label justifications:

P2: The PR fixes bounded memory growth in a Slack channel worker path with limited blast radius and no evidence of an urgent outage.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit 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 PR body includes copied terminal output for an after-fix runtime cache proof plus focused regression/static checks, which is enough for this non-visual process-cache behavior.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied terminal output for an after-fix runtime cache proof plus focused regression/static checks, which is enough for this non-visual process-cache behavior.

Evidence reviewed

PR surface:

Source +25, Tests +49. Total +74 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	32	7	+25
Tests	1	49	0	+49
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	81	7	+74

What I checked:

Current main behavior: Current main stores Slack conversation info in a module-level Map and writes every cached account/channel result without any size bound or eviction. (extensions/slack/src/channel-type.ts:16, 942b44966107)
PR implementation: The PR head adds a 1024-entry cap, promotes cache hits by delete/set, and evicts the oldest key before inserting a new distinct cache entry once the cap is reached. (extensions/slack/src/channel-type.ts:16, 56aa859e74ce)
Regression coverage: The added test fills 1024 entries, promotes the first key, inserts one extra key, verifies the least-recently-used entry is recomputed, and verifies the promoted key remains cached. (extensions/slack/src/channel-type.test.ts:181, 56aa859e74ce)
Runtime call path: Slack outbound session routing calls resolveSlackConversationInfo for D-prefixed targets and resolveSlackChannelType for G-prefixed targets, so the changed cache sits on a real Slack channel worker path. (extensions/slack/src/channel.ts:363, 942b44966107)
Sibling cache pattern: The Slack write-client cache already uses Map delete/set promotion and oldest-key eviction, so the PR follows an established local bounded process-cache pattern. (extensions/slack/src/client.ts:41, 942b44966107)
Feature provenance: The Slack conversation-info helper was introduced in the Slack DM mirror-route canonicalization work, which added the same helper and tests that this PR now bounds. (extensions/slack/src/channel-type.ts:15, 50c77f23c1c1)

Likely related people:

steipete: Landed the corrected Slack conversation-info helper in commit 50c77f2 and appears repeatedly in recent Slack channel/client history, including client cache and session-routing work. (role: recent Slack session-routing contributor and merger; confidence: high; commits: 50c77f23c1c1, d399ac74f752, b69e3b633b28; files: extensions/slack/src/channel-type.ts, extensions/slack/src/channel.ts, extensions/slack/src/client.ts)
bek91: Authored the earlier Slack DM mirror-route PR that expanded channel-type lookup into conversation metadata before a corrected maintainer commit landed the helper. (role: source PR author for adjacent helper behavior; confidence: medium; commits: a910de2a9d04, 50c77f23c1c1; files: extensions/slack/src/channel-type.ts, extensions/slack/src/channel.ts, extensions/slack/src/channel.test.ts)
Takhoffman: GitHub path history for channel-type shows prior work on Slack default-account behavior in the same helper area. (role: channel-type helper contributor; confidence: medium; commits: 5f1736266709; files: extensions/slack/src/channel-type.ts)
zhangguiping-xydt: Beyond authoring this PR, recent merged history shows adjacent outbound recovery work touching Slack channel behavior, so they have relevant area context. (role: recent adjacent Slack/outbound contributor; confidence: low; commits: 741d9524bec2; files: extensions/slack/src/channel.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.

vincentkoc · 2026-07-07T12:54:48Z

Land-ready maintainer repair completed. The contributor fix now reuses the shared pruneMapToMaxSize helper while preserving LRU hit promotion and the 1024-entry bound. Focused Slack proof passed 7/7 tests; formatting and diff checks passed; exact repaired content passed check:changed on Testbox-through-Crabbox tbx_01kwy9f5yj11g6md3fwxvpv3yr; fresh autoreview reported no actionable findings at 0.91 confidence; exact-head CI run 28867053630 completed successfully with zero failures. Repaired head: 9fe0d98.

vincentkoc · 2026-07-07T12:55:06Z

Merged via squash.

Prepared head SHA: 9fe0d98e82b63e53e7a8e26462cd7bbad5848c43
Landed commit: 4e29b2f5ab4bd917382de069a148d103c35413f0

openclaw-barnacle Bot added channel: slack Channel integration: slack size: S labels Jul 7, 2026

sunlit-deng mentioned this pull request Jul 7, 2026

fix(infra): cap session-maintenance-warning dedupe cache with LRU eviction #101643

Merged

vincentkoc force-pushed the fix/problem-slack-conversation-cache-lru branch from 56aa859 to 0b364df Compare July 7, 2026 12:28

fix(slack): bound conversation info cache

9fe0d98

vincentkoc force-pushed the fix/problem-slack-conversation-cache-lru branch from 0b364df to 9fe0d98 Compare July 7, 2026 12:43

vincentkoc self-assigned this Jul 7, 2026

vincentkoc merged commit 4e29b2f into openclaw:main Jul 7, 2026
94 checks passed

clawsweeper Bot mentioned this pull request Jul 7, 2026

fix(channels): prevent metadata caches from growing without bound #101650

Merged

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

📡 Upstream Digest — 2026-07-07 14:10 UTC curtismercier/openclaw-mods#1230

Open

This was referenced Jul 7, 2026

refactor: flip sessions and transcripts to sqlite storage #98236

Merged

test-issue #101870

Closed

github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 8, 2026

fix(slack): bound conversation info cache (openclaw#101562)

55a1cfc

giodl73-repo pushed a commit to giodl73-repo/openclaw that referenced this pull request Jul 8, 2026

fix(slack): bound conversation info cache (openclaw#101562)

4e453ce

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(slack): conversation lookups no longer grow cache without bound#101562

fix(slack): conversation lookups no longer grow cache without bound#101562
vincentkoc merged 1 commit into
openclaw:mainfrom
zhangguiping-xydt:fix/problem-slack-conversation-cache-lru

zhangguiping-xydt commented Jul 7, 2026

Uh oh!

clawsweeper Bot commented Jul 7, 2026 •

edited

Loading

Uh oh!

vincentkoc commented Jul 7, 2026

Uh oh!

Uh oh!

vincentkoc commented Jul 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

zhangguiping-xydt commented Jul 7, 2026

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Uh oh!

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

Uh oh!

Uh oh!

vincentkoc commented Jul 7, 2026

Uh oh!

Uh oh!

vincentkoc commented Jul 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

clawsweeper Bot commented Jul 7, 2026 •

edited

Loading