PR #71648: fix(mcp): bound pendingClaudePermissions / pendingApprovals via TTL sweeper + close clear

• Repository: openclaw/openclaw
• Author: Feelw00
• State: open | merged: False
• Link: https://github.com/openclaw/openclaw/pull/71648

Description (problem / solution / changelog)

Summary

• Problem: OpenClawChannelBridge (src/mcp/channel-bridge.ts) holds two instance-bound pending Maps — pendingClaudePermissions (L50) and pendingApprovals (L51) — that lack TTL/sweeper, close-clear, and cap. Sibling collections in the same class (queue cap=1000 at L355, pendingWaiters close-clear at L148 + per-waiter setTimeout fallback at L265) are all bounded; only these two are not.
• Why it matters: long-running openclaw mcp serve accumulates entries on (a) missed Claude permission replies (operator typos / cross-channel responses / --dangerously-skip-permissions) and (b) gateway WebSocket drops that silence *.approval.resolved frames (onClose at L122-124 is reject-only, no missed-event catchup).
• What changed: lazy-start a 5-min sweepPendingExpired() interval, .unref()'d so it never keeps the process alive. Per-Map TTL — 1h for Claude permissions (anchored at createdAtMs), respects payload.expiresAtMs for approvals (falls back to trackedAtMs + 30min when absent). close() now clears both Maps and stops the sweeper. Handler entry early-returns when this.closed to prevent post-close ghost writes.
• What did NOT change: public listPendingApprovals shape, message protocols, or any other class collection. cap/FIFO is intentionally split to a follow-up PR (one-thing-per-PR; sweeper covers the slow leak, cap addresses adversarial bursts).

Change Type (select all)

• Bug fix
• Feature
• Refactor required for the fix
• Docs
• Security hardening
• Chore/infra

Scope (select all touched areas)

• Memory / storage
• Integrations
• Gateway / orchestration
• Skills / tool execution
• Auth / tokens
• API / contracts
• UI / DX
• CI/CD / infra

Linked Issue/PR

Closes #71646.

• This PR fixes a bug or regression

Root Cause

Upstream commit history shows channel-bridge.ts evolving across refactor/seam-split commits over the last 6 weeks, but no commit added a TTL/sweep/cap guard for these two pending Maps. The asymmetry is structural:

pendingClaudePermissions only deletes when the operator reply matches /^(yes|no)\s+([a-km-z]{5})$/i AND has(requestId) is true (L455-459) — a single conditional-edge cleanup. pendingApprovals only deletes on the matching *.resolved frame (L389) — another single conditional-edge. No primary unconditional cleanup path exists.

Regression Test Plan

New file src/mcp/channel-bridge.test.ts (7 it blocks, fake timers, no production-branch divergence):

1. pendingClaudePermissions evicted by sweeper at TTL boundary
2. pendingApprovals evicted at expiresAtMs
3. pendingApprovals evicted at default TTL fallback when expiresAtMs is absent
4. pendingApprovals evicted even when both createdAtMs and expiresAtMs are absent (regression test for the now-anchored fallback bug caught in pre-PR review)
5. close() clears both Maps, stops the sweeper interval, and vi.getTimerCount() === 0
6. handleClaudePermissionRequest is a no-op after close() — prevents post-close ghost writes
7. Sweeper interval is not started before any pending entry is added (lazy-init)

channel-server.test.ts:366 fixture updated to the new {request, createdAtMs} wrapper shape.

User-visible / Behavior Changes

• Long-running openclaw mcp serve sees pending Map sizes converge to a steady state instead of growing monotonically.
• Operators who never reply to a Claude permission_request no longer keep the entry forever; it expires after 1h and the next permission_request for the same tool starts fresh.
• Approvals announced over a dropped gateway WebSocket are auto-evicted at expiresAtMs (or trackedAtMs + 30min if the gateway omitted expiresAtMs).
• No protocol surface changes; client tools see no difference.

Security Impact

None. The fix is purely about bounded memory growth in an opt-in long-running daemon. No auth / scope / token / sandbox surface touched. CODEOWNERS protected paths (/src/cron/service/jobs.ts, /src/cron/stagger.ts, /src/agents/*auth*, /src/agents/sandbox*) untouched.

Repro + Verification

Environment

• pnpm 10.33.0 / Node (project default) / vitest 4.1.5
• Worktree: clean checkout of upstream/main + this branch
• Platform: macOS (Darwin 25.4.0)

Steps

pnpm install
pnpm check                                                       # type + lint
pnpm build                                                       # build
pnpm exec vitest run src/mcp/channel-bridge.test.ts              # new tests
pnpm exec vitest run src/mcp/channel-server.test.ts              # existing mcp tests after fixture sync
pnpm exec vitest run --config test/vitest/vitest.unit.config.ts  # unit project regression

Expected

All green. New file 7/7. Existing channel-server 6/6. Unit project 1575/1575 (was 1573 pre-fix; +2 from new file's count vs prior file count).

Actual

src/mcp/channel-bridge.test.ts: Tests 7 passed (7), 640ms
src/mcp/channel-server.test.ts: Tests 6 passed (6)
unit project: Test Files 189 passed (189), Tests 1575 passed (1575), 6.48s
pnpm check: clean
pnpm build: clean

Evidence

• src/mcp/channel-bridge.ts:50-51 — pre-fix declarations of the two leaking Maps
• src/mcp/channel-bridge.ts:136-152 — pre-fix close() body (only pendingWaiters.clear())
• src/mcp/channel-bridge.ts:355-356 — pre-existing queue cap pattern (sibling reference)
• src/mcp/channel-bridge.ts:265-267 — pre-existing per-waiter setTimeout (sibling reference)
• Diff: ~80 lines prod (channel-bridge.ts +89 -4) + 9 lines fixture sync (channel-server.test.ts) + 173 lines new test file

Human Verification

I confirm I have reviewed every diff hunk, ran the build/check/test suites locally, and the regression tests fail on upstream/main (no fix) and pass on this branch (with fix).

Review Conversations

This is a fresh PR. No prior review threads.

Compatibility / Migration

Fully backwards-compatible. No public API, no protocol field, no config surface changed. The internal Map value shape ({request, createdAtMs} wrapper, {approval, trackedAtMs} wrapper) is private — only the test that previously cast through Record<string, unknown> was updated to match.

Risks and Mitigations

• Risk: TTL constants (PENDING_CLAUDE_PERMISSION_TTL_MS=1h, PENDING_APPROVAL_DEFAULT_TTL_MS=30min, PENDING_SWEEP_INTERVAL_MS=5min) are hardcoded. Mitigation: chosen for low operator-disruption (1h is longer than typical permission-think time; 30min matches existing expiresAtMs field intent). Happy to migrate to a config surface in a follow-up if requested.
• Risk: PR #56420 (sessionKey binding, OPEN) touches the same set() line in handleClaudePermissionRequest. Mitigation: line-level rebase only; semantic axes are orthogonal (binding vs leak). Whichever PR merges first, the other rebases cleanly.
• Risk: cap/FIFO is not in this PR — adversarial bursts within a single sweep tick (5min) could still grow large in pathological cases. Mitigation: deferred to a follow-up PR (one-thing-per-PR). Sweeper closes the slow leak that motivated the report; cap is a separate concern.

🤖 AI-Assisted (openclaw-audit pipeline). 5-agent post-harness cross-review (proceed: 4 real + 1 fix-insufficient → cap deferred to follow-up). 3-agent pre-PR cross-review round 1 caught a fallback-recalculation bug in the original sweeper (createdAtMs ?? now re-anchored expiry on every tick) → fixed via trackedAtMs instance bookkeeping; round 2 cleared 3/3 real-problem-real-fix.

Changed files

• src/mcp/channel-bridge.test.ts (added, +202/-0)
• src/mcp/channel-bridge.ts (modified, +85/-16)
• src/mcp/channel-server.test.ts (modified, +6/-3)