Repository navigation
fix(mcpx): cache — traversal rejection, Put self-eviction, eviction lease re-check (#2566) - #2838
Conversation
…heck leases at eviction removal (#2566)
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 10 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Fixes #2566 — the three verified Codex P2 findings on merged PR #2555 (mcpx
internal/cache, #1908 Approach C), each with a regression test:Cache.Path/Dirnow reject kind/key segments that are empty, dot components, or contain a path separator (returning""), and every operation validates up front:Put/Remove/Listreturn an error,Getreports a miss.Removeadditionally verifies the resolved target is exactly one entry directly inside the kind directory before itsos.RemoveAll. Test:TestTraversalNamesRejected(a../attempt against a sentinel directory outside the root fails and the sentinel survives).Putnow evicts withkeepset to the entry it just stored. An entry larger thanMaxBytesis kept (the bound is exceeded by that entry alone) instead of being deleted by its ownPutwhilePutreported success; older entries are evicted first. Test:TestPutNeverEvictsItsOwnEntry(the 2-byte payload /MaxBytes: 1repro).removeAll(shared byEvictandPrune) re-checksLeased()immediately before each removal, so an entry leased after the victim scan is skipped. Test:TestRemoveAllRechecksLeaseAfterSelection(deterministic scan → lease → evict case incache_internal_test.go).The three Codex threads on PR #2555 were already replied to from queue task 3148 with the #2566 link; this PR is the fix those replies promised.
Tests:
go test ./internal/cache/passes on the laptop (full package, including the three new regression tests); fullpkgs/mcpxsuite runs in CI on this PR.Queue task 3817 (repo-health-code-pilot).
Need help on this PR? Tag
@codesmith-botwith what you need.