Skip to content

Clarify session eviction cleanup guarantees - #6833

Merged
JAORMX merged 1 commit into
mainfrom
docs/session-eviction-guarantees-6807
Oct 15, 2026
Merged

JAORMX merged 1 commit into
mainfrom
docs/session-eviction-guarantees-6807

Conversation

@JAORMX

@JAORMX JAORMX commented Oct 15, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • To avoid promising teardown completion that the shared eviction drain cannot guarantee, clarify that callbacks run without cache mutexes held by their invoking goroutine and may be drained by another operation.
  • To keep session lifecycle documentation consistent with Close terminated vMCP sessions instead of waiting for eviction #6807, distinguish eager local cache removal from completion of backend cleanup and update the older termination test's lazy-eviction comments.

Addresses the three non-blocking review comments on #6807. This follow-up changes comments only; synchronization, teardown behavior, and test assertions are unchanged.

Type of change

  • Bug fix
  • New feature
  • Refactoring (no behavior change)
  • Dependency update
  • Documentation
  • Other (describe):

Test plan

  • Unit tests (task test)
  • E2E tests (task test-e2e)
  • Linting (task lint-fix)
  • Manual testing (describe below)

Reviewed the documented callback ownership and return-time guarantees against bufferEvicted, drainEvictions, cache mutation paths, and Manager.Terminate. Ran git diff --check and a diff audit confirming that every changed line is a Go comment or blank line. Local unit/E2E tests were not run because executable code and assertions are unchanged.

Does this introduce a user-facing change?

No. Documentation only.

Implementation plan

Approved implementation plan

Merge the focused session-retention fix in #6807 without holding it for documentation polish, then address its three non-blocking comments in a separate PR:

  1. Qualify termination's backend-teardown completion guarantee.
  2. Clarify callback lock ownership and shared-drain latency semantics.
  3. Update the older test comments that still describe lazy eviction.

Distinguish eager cache removal from callback completion under shared eviction draining. Correct lock ownership guarantees and stale lazy-eviction test comments after #6807 without changing behavior.

Co-authored-by: Mecatl <noreply@mecatl.dev>
@github-actions github-actions Bot added the size/S Small PR: 100-299 lines changed label Oct 15, 2026
@codecov

codecov Bot commented Oct 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.63%. Comparing base (607cbe8) to head (81ee385).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6833      +/-   ##
==========================================
+ Coverage   79.62%   79.63%   +0.01%     
==========================================
  Files         815      815              
  Lines       82965    82965              
==========================================
+ Hits        66060    66073      +13     
+ Misses      16900    16887      -13     
  Partials        5        5              

☔ View full report in Codecov by Harness.
📢 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.

@JAORMX
JAORMX merged commit b7886b7 into main Oct 15, 2026
47 checks passed
@JAORMX
JAORMX deleted the docs/session-eviction-guarantees-6807 branch October 15, 2026 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/S Small PR: 100-299 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants