Skip to content

fix(agents): do not refresh lastUsedAt on MCP lease release#91124

Merged
clawsweeper[bot] merged 2 commits into
openclaw:mainfrom
openperf:fix/91075-mcp-idle-sweep-release-timestamp
Jun 8, 2026
Merged

fix(agents): do not refresh lastUsedAt on MCP lease release#91124
clawsweeper[bot] merged 2 commits into
openclaw:mainfrom
openperf:fix/91075-mcp-idle-sweep-release-timestamp

Conversation

@openperf

@openperf openperf commented Jun 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Summary

  • Problem: Issue MCP runtime idle sweep defeated by releaseLease resetting lastUsedAt #91075 reports MCP server processes accumulating linearly over gateway uptime — 24 processes / 3.3 GB observed after ~3 h with 4 cron agents running at 5–15 min intervals. sweepIdleRuntimes never reclaims any of them.
  • Root Cause: acquireLease() in createSessionMcpRuntime returns a release callback that unconditionally sets lastUsedAt = Date.now(). The idle sweep evicts a runtime only when nowMs - runtime.lastUsedAt >= idleTtlMs and activeLeases === 0. When a cron agent holds a lease for longer than sessionIdleTtlMs, releasing that lease resets the clock to the moment of release; the very next sweep finds the runtime within TTL again and keeps it. Because cron agents re-lease before the next full TTL cycle elapses, the sweep never fires. The accumulated runtimes — including their MCP child processes and SSE/HTTP handles — are never disposed.
  • Fix: Remove lastUsedAt = Date.now() from the release callback. markUsed() already stamps the clock at the correct points: inside materializeBundleMcpToolsForRun when tools are prepared and before every tool/resource/prompt execution. Releasing a lease is not "using" the runtime and must not extend the TTL window.
  • What changed:
    • src/agents/agent-bundle-mcp-runtime.ts — remove lastUsedAt = Date.now() from acquireLease() release callback; add a one-line comment explaining the invariant.
    • src/agents/agent-bundle-mcp-runtime.test.ts — update the existing idle-eviction test mock to match corrected production behavior (remove mirrored lastUsedAt = now from the mock release callback); add new regression test evicts immediately after release when TTL already elapsed during active lease.
  • What did NOT change (scope boundary):
    • markUsed() call sites in agent-bundle-mcp-materialize.ts are unchanged; the TTL window resets correctly at materialization and tool execution.
    • sweepIdleRuntimes() logic, active-lease guard, and dispose path are unchanged.
    • Config surface unchanged (no schema, defaults, doctor migrations, or docs/reference/config).
    • Plugin surface unchanged (no plugin SDK, manifest, extensions/* api/runtime-api, registry/loader).
    • Gateway protocol unchanged.

Reproduction

  1. Start gateway with cron agents that reuse the same session key at intervals shorter than mcp.sessionIdleTtlMs (default 600 s).
  2. Monitor MCP child-process count over several hours — it grows linearly, one per cron fire, and idle sweep never reduces it.
  3. Before this PR: the lease release resets lastUsedAt to now; the next sweepIdleRuntimes call finds nowMs - lastUsedAt < idleTtlMs and skips the runtime; process accumulation is unbounded.
  4. After this PR: releasing a lease leaves lastUsedAt at its last genuine use time; a runtime held past its TTL is evicted on the first sweep after the final lease is released, without requiring any further clock advance.

Real behavior proof

Behavior addressed (#91075): MCP server processes no longer accumulate after the patch; a runtime whose TTL elapsed while a lease was active is evicted on the next sweepIdleRuntimes call after the lease is released.

Real environment tested (Linux, Node 22 — Vitest against the production createSessionMcpRuntime, acquireLease, and sweepIdleRuntimes implementations in agent-bundle-mcp-runtime.ts): tests drive the real manager and runtime factory with a controlled clock; no mocking of the idle-sweep or dispose logic.

Exact steps or command run after this patch: node scripts/run-vitest.mjs src/agents/agent-bundle-mcp-runtime.test.ts -t "evicts idle runtimes|evicts immediately after release" — 4 passed; pnpm format:fix on changed files; node scripts/run-oxlint.mjs on changed files — clean.

Evidence after fix: Vitest: 4 passed, 0 failed; autoreview: no accepted/actionable findings, correctness 0.97.

Observed result after fix: in the new regression test, the clock does not advance after the lease is released — yet sweepIdleRuntimes returns 1 and dispose is called, confirming the runtime is evicted immediately rather than after an additional full TTL cycle.

What was not tested: live gateway cron scenario with real MCP child processes; the sweep logic and dispose path are covered by the test harness against production functions.

Repro confirmation: the new regression test (evicts immediately after release when TTL already elapsed during active lease) fails on the unfixed tree because the mock release callback refreshes lastUsedAt and the sweep returns 0 instead of 1; it passes with the fix, directly covering the reported accumulation path.

Risk / Mitigation

  • Risk: a caller relying on the release callback to refresh the TTL window silently loses that extension. Mitigation: no such caller exists; markUsed() is the only documented mechanism and is already invoked at acquireLease time inside materializeBundleMcpToolsForRun.
  • Risk: an active runtime disposed prematurely if activeLeases and lastUsedAt are out of sync. Mitigation: sweepIdleRuntimes still skips any runtime with activeLeases > 0; the TTL is only evaluated after all leases are released.

Change Type (select all)

  • Bug fix

Scope (select all touched areas)

  • Agents / embedded runner

Linked Issue/PR

Fixes #91075

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: S labels Jun 7, 2026
@clawsweeper

clawsweeper Bot commented Jun 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: passed. Reviewed June 7, 2026, 10:06 PM ET / 02:06 UTC.

Summary
The PR removes release-time lastUsedAt refresh from session MCP runtime lease cleanup and adds regression tests for idle eviction after a lease expires while active.

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

Reproducibility: yes. from source inspection: current main refreshes lastUsedAt in the release callback, and the idle sweep uses that timestamp after active leases drop to zero. I did not execute the focused Vitest in this read-only review.

Review metrics: none identified.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
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.

Risk before merge

  • [P1] The PR body says a live multi-hour gateway cron run with real MCP child processes was not tested; the focused production-function tests cover the lease/sweep invariant instead.

Maintainer options:

  1. Decide the mitigation before merge
    Land the focused lease-release fix after required CI/automerge gates, keeping markUsed() as the runtime use signal and activeLeases as the sweep safety guard.
  2. 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 the PR is already a focused implementation with no blocking findings in this read-only review.

Security
Cleared: The diff is limited to in-process MCP runtime lease cleanup and tests, with no dependency, workflow, secret, install, or package-resolution changes.

Review details

Best possible solution:

Land the focused lease-release fix after required CI/automerge gates, keeping markUsed() as the runtime use signal and activeLeases as the sweep safety guard.

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

Yes from source inspection: current main refreshes lastUsedAt in the release callback, and the idle sweep uses that timestamp after active leases drop to zero. I did not execute the focused Vitest in this read-only review.

Is this the best way to solve the issue?

Yes; this is the narrowest maintainable fix because release should only end the lease, while materialization and MCP tool/resource/prompt execution already call markUsed() for genuine runtime use.

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 75c1790b504c.

Label changes

Label changes:

  • add status: 🚀 automerge armed: This PR is in ClawSweeper's automerge lane. Not applicable: The PR author has MEMBER association, so the external-contributor real behavior proof gate is not applicable; the PR body still reports focused production-function Vitest, format, and oxlint proof.
  • remove status: 👀 ready for maintainer look: Current PR status label is status: 🚀 automerge armed.

Label justifications:

  • P1: The linked bug reports MCP child processes and memory accumulating during recurring agent runs, which can degrade active agent availability.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 🚀 automerge armed: This PR is in ClawSweeper's automerge lane. Not applicable: The PR author has MEMBER association, so the external-contributor real behavior proof gate is not applicable; the PR body still reports focused production-function Vitest, format, and oxlint proof.
Evidence reviewed

PR surface:

Source 0, Tests +74. Total +74 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 1 1 0
Tests 1 76 2 +74
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 77 3 +74

What I checked:

Likely related people:

  • vincentkoc: git blame and release-tag inspection attribute the session MCP runtime file and the current release-time lastUsedAt refresh to Vincent Koc-authored commits, including dcba17d0193963893f6a50862112455291a2f374 and the v2026.6.1 release baseline. (role: introduced behavior and recent area contributor; confidence: high; commits: dcba17d01939, 2e08f0f4221f; files: src/agents/agent-bundle-mcp-runtime.ts, src/agents/agent-bundle-mcp-runtime.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.

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P1 High-priority user-facing bug, regression, or broken workflow. labels Jun 7, 2026
@openperf openperf force-pushed the fix/91075-mcp-idle-sweep-release-timestamp branch from 244d164 to 3535e5c Compare June 7, 2026 09:19
@clawsweeper clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jun 7, 2026
@Takhoffman

Takhoffman commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

@clawsweeper automerge

@clawsweeper

clawsweeper Bot commented Jun 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞✅
ClawSweeper merged this PR after the passing review.

Source: clawsweeper[bot]
Feedback: structured ClawSweeper verdict: pass (sha=c9144789fd8bc3d629b68cbc7a00a18a57179dab)
Merge status: merged by ClawSweeper automerge
Merged at: 2026-06-08T02:08:29Z
Merge commit: f2530de8320e

What merged:

  • The PR removes release-time lastUsedAt refresh from session MCP runtime lease cleanup and adds regression tests for idle eviction after a lease expires while active.
  • PR surface: Source 0, Tests +74. Total +74 across 2 files.
  • Reproducibility: yes. from source inspection: current main refreshes lastUsedAt in the release callback, a ... timestamp after active leases drop to zero. I did not execute the focused Vitest in this read-only review.

Automerge notes:

  • PR branch already contained follow-up commit before automerge: fix(agents): do not refresh lastUsedAt on MCP lease release

The automerge loop is complete.

Automerge progress:

  • 2026-06-08 02:01:29 UTC review queued c9144789fd8b (after repair)
  • 2026-06-08 02:07:19 UTC review passed c9144789fd8b (structured ClawSweeper verdict: pass (sha=c9144789fd8bc3d629b68cbc7a00a18a57179...)
  • 2026-06-08 02:07:31 UTC review queued c9144789fd8b (queued)
  • 2026-06-08 02:08:31 UTC merged c9144789fd8b (merged by ClawSweeper automerge)

@clawsweeper clawsweeper Bot added clawsweeper:automerge Maintainer opted this PR into bounded ClawSweeper-reviewed automerge clawsweeper:human-review Needs maintainer review before ClawSweeper can continue labels Jun 7, 2026
@clawsweeper

clawsweeper Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

🦞✅
ClawSweeper is pausing this repair loop for human review.

Source: clawsweeper[bot]
Reason: - No ClawSweeper repair lane is needed; the previous narrow test-coverage blocker is addressed in the force-pushed patch and the remaining step is normal maintainer/CI review.; Cleared: The diff only changes MCP runtime lease timestamp behavior and colocated tests; I found no concrete security or supply-chain regression. (sha=3535e5c70fb0a8c24b6b3c8b345dd1e5cea57210)

Why human review is needed:
This item has security-sensitive risk. ClawSweeper is pausing instead of making an autonomous change that could affect trust, credentials, permissions, or exposure.

What the maintainer can do as a next step:
If the maintainer accepts the current risk and wants ClawSweeper to continue merge gates, comment @clawsweeper approve. If the security-sensitive detail still needs changes, describe the safe path or push the fix, then comment @clawsweeper automerge. If the risk should not be automated, keep the PR paused for manual review or comment @clawsweeper stop.

I added clawsweeper:human-review and left the final call with a maintainer.

@Takhoffman

Takhoffman commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

@clawsweeper approve

@clawsweeper

clawsweeper Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Command router queued. I will update this comment with the next step.

@clawsweeper clawsweeper Bot removed the clawsweeper:human-review Needs maintainer review before ClawSweeper can continue label Jun 8, 2026
@clawsweeper clawsweeper Bot force-pushed the fix/91075-mcp-idle-sweep-release-timestamp branch from 3535e5c to c914478 Compare June 8, 2026 02:01
@clawsweeper clawsweeper Bot added status: 🚀 automerge armed This PR is in ClawSweeper's automerge lane. and removed status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jun 8, 2026
@clawsweeper clawsweeper Bot merged commit f2530de into openclaw:main Jun 8, 2026
169 of 171 checks passed
@Haderach-Ram Haderach-Ram mentioned this pull request Jun 8, 2026
Open
@github-actions github-actions Bot mentioned this pull request Jun 8, 2026
Open
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jun 8, 2026
@openperf @clawsweeper @Takhoffman
fix(agents): do not refresh lastUsedAt on MCP lease release (openclaw…
7dbe7d5 
…#91124)

Summary:
- The PR removes release-time `lastUsedAt` refresh from session MCP runtime lease cleanup and adds regression tests for idle eviction after a lease expires while active.
- PR surface: Source 0, Tests +74. Total +74 across 2 files.
- Reproducibility: yes. from source inspection: current main refreshes `lastUsedAt` in the release callback, a ...  timestamp after active leases drop to zero. I did not execute the focused Vitest in this read-only review.

Automerge notes:
- PR branch already contained follow-up commit before automerge: fix(agents): do not refresh lastUsedAt on MCP lease release

Validation:
- ClawSweeper review passed for head c914478.
- Required merge gates passed before the squash merge.

Prepared head SHA: c914478
Review: openclaw#91124 (comment)

Co-authored-by: openperf <16864032@qq.com>
Co-authored-by: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com>
Co-authored-by: clawsweeper[bot] <274271284+clawsweeper[bot]@users.noreply.github.com>
Approved-by: takhoffman
Co-authored-by: takhoffman <781889+takhoffman@users.noreply.github.com>
This was referenced Jun 10, 2026
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

agents Agent runtime and tooling clawsweeper:automerge Maintainer opted this PR into bounded ClawSweeper-reviewed automerge P1 High-priority user-facing bug, regression, or broken workflow. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. size: S status: 🚀 automerge armed This PR is in ClawSweeper's automerge lane.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

MCP runtime idle sweep defeated by releaseLease resetting lastUsedAt

2 participants

@openperf @Takhoffman