keyv - fix: a namespaced clear() fails instead of wiping a store it can't scope - #2186
Merged
Merged
Conversation
…an't scope KeyvMemoryAdapter and KeyvBridgeAdapter fell back to the store's own clear() when they had a namespace but no way to find its keys: a Map-like store without keys(), or an async store without iterator() that doesn't manage its own namespace. That deleted every namespace's entries, though the docs promise that clearing `users` doesn't touch `cache`. Both adapters now throw instead, and Keyv's clear() turns that into an `error` event (a rejection with no listener), so nothing is deleted. clear() without a namespace still empties the store, which keeps a deliberate full wipe available. Stores that can be scoped (a Map, LRUs with keys(), adapters with an iterator() or their own namespace) behave as before. v5 called the store's clear() directly and deleted everything, so the migration guide and the keyv-migrate skill references get a section on the change, and the README and the namespace, Map/LRU, legacy adapter and method docs describe the new behavior. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wm1wtCGzQJEyYZdekgNVge
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. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2186 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 56 56
Lines 5797 5798 +1
Branches 995 998 +3
=========================================
+ Hits 5797 5798 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
2 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Bug fix that changes behavior, with docs. It's the second pre-GA fix, after jaredwray/keyv#2185.
KeyvMemoryAdapter.clear()andKeyvBridgeAdapter.clear()fell back to the store's ownclear()when a namespace was set but they had no way to find its keys. That happened with aMap-like store withoutkeys(), and with an async store withoutiterator()that doesn't manage its own namespace. The fallback deleted every namespace's entries, though the docs promise that clearingusersdoesn't touchcache.Changes
Keyv.clear()already turns an adapter error into anerrorevent, so nothing is deleted: the call resolves when a listener is attached and rejects when none is.clear()without a namespace still empties the store, so a deliberate full wipe is still available. Stores that can be scoped behave as before: aMap, LRUs withkeys(), stores withiterator(), adapters that manage their own namespace, and every v6 adapter.erroremitted with a listener, a rejection without one, the store left alone, and a full clear without a namespace. They fail onmain.Keyv.clear()calledstore.clear()directly and wiped everything, so the migration guide gets a section and table of contents entry, and thekeyv-migrateskill references (v5-to-v6.md,custom-adapters.md) note the change, per AGENTS.md.Verification
pnpm testincore/keyv: lint clean, 19 files and 367 tests pass, and every branch inbridge.tsandmemory.tsis covered.keyv, these suites also pass: bigmap, test-suite, the three compression adapters, both encryption adapters, both serializers, sqlite, redis (against local Redis), and the website's docs and skill validation.🤖 Generated with Claude Code
https://claude.ai/code/session_01Wm1wtCGzQJEyYZdekgNVge
Generated by Claude Code