Skip to content

Add LRU capacity to ValidatingCache, remove sentinel pattern, add storage Update#4731

Merged
yrobla merged 2 commits intomainfrom
revert-4730-issue-4494
Apr 13, 2026
Merged

Add LRU capacity to ValidatingCache, remove sentinel pattern, add storage Update#4731
yrobla merged 2 commits intomainfrom
revert-4730-issue-4494

Conversation

@yrobla
Copy link
Copy Markdown
Contributor

@yrobla yrobla commented Apr 10, 2026

Summary

Re-applies the changes from #4669 which were reverted in #4730.

  • Reintroduces pkg/cache.ValidatingCache[K,V]: a fully-typed, capacity-bounded LRU cache using mutex+map+container/list
  • Removes the terminatedSentinel / sentinel+rollback pattern
  • Adds DataStorage.Update (SET XX semantics) to close the TOCTOU resurrection window
  • Adds FactoryConfig.CacheCapacity to wire the LRU limit through sessionmanager.New
  • Cache tests live in pkg/cache/validating_cache_test.go

Fixes #4494

Type of change

  • New feature

Test plan

  • Unit tests (task test)
  • Linting (task lint-fix)

🤖 Generated with Claude Code

Large PR Justification

This is the revert of an existing PR. Cannot be split.

…pattern, add storage Update" (#4730)"

This reverts commit acc1f66.
@github-actions github-actions bot added the size/XL Extra large PR: 1000+ lines changed label Apr 10, 2026
Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Large PR Detected

This PR exceeds 1000 lines of changes and requires justification before it can be reviewed.

How to unblock this PR:

Add a section to your PR description with the following format:

## Large PR Justification

[Explain why this PR must be large, such as:]
- Generated code that cannot be split
- Large refactoring that must be atomic
- Multiple related changes that would break if separated
- Migration or data transformation

Alternative:

Consider splitting this PR into smaller, focused changes (< 1000 lines each) for easier review and reduced risk.

See our Contributing Guidelines for more details.


This review will be automatically dismissed once you add the justification section.

@yrobla yrobla changed the title Revert "Revert "Add LRU capacity to ValidatingCache, remove sentinel pattern, add storage Update"" Add LRU capacity to ValidatingCache, remove sentinel pattern, add storage Update Apr 10, 2026
@github-actions github-actions bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Apr 10, 2026
jerm-dro
jerm-dro previously approved these changes Apr 10, 2026
Copy link
Copy Markdown
Contributor

@jerm-dro jerm-dro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some failing tests that seem minor.

I'm assuming there's no major changes here compared to the original, but LMK if that's not true

@github-actions github-actions bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Apr 13, 2026
@yrobla yrobla force-pushed the revert-4730-issue-4494 branch from 76455ef to f917c29 Compare April 13, 2026 07:43
@github-actions github-actions bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Apr 13, 2026
@yrobla yrobla force-pushed the revert-4730-issue-4494 branch from f917c29 to aecdae1 Compare April 13, 2026 07:55
@github-actions github-actions bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Apr 13, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 13, 2026

Codecov Report

❌ Patch coverage is 78.57143% with 33 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.86%. Comparing base (acc1f66) to head (aecdae1).
⚠️ Report is 25 commits behind head on main.

Files with missing lines Patch % Lines
pkg/vmcp/server/sessionmanager/session_manager.go 57.44% 10 Missing and 10 partials ⚠️
pkg/cache/validating_cache.go 80.00% 5 Missing and 5 partials ⚠️
...kg/transport/session/session_data_storage_redis.go 82.35% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4731      +/-   ##
==========================================
+ Coverage   68.75%   68.86%   +0.11%     
==========================================
  Files         518      517       -1     
  Lines       54826    54470     -356     
==========================================
- Hits        37693    37512     -181     
+ Misses      14239    14085     -154     
+ Partials     2894     2873      -21     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions github-actions bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Apr 13, 2026
@github-actions github-actions bot dismissed their stale review April 13, 2026 08:21

Large PR justification has been provided. Thank you!

@github-actions
Copy link
Copy Markdown
Contributor

✅ Large PR justification has been provided. The size review has been dismissed and this PR can now proceed with normal review.

@yrobla yrobla merged commit 6cac96a into main Apr 13, 2026
42 checks passed
@yrobla yrobla deleted the revert-4730-issue-4494 branch April 13, 2026 09:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XL Extra large PR: 1000+ lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

vmcp: address RestorableCache capacity limit and sentinel pattern complexity (follow-up to #4464)

4 participants