fix: harden search/forget behavior and skip capture for memory-management turns#25
Conversation
| const preview = limitText(target.content || target.memory || "", 100) | ||
| if (!deleted.forgotten) { | ||
| return { | ||
| success: false, | ||
| message: `Unable to confirm forgetting: "${preview}"`, | ||
| } |
There was a problem hiding this comment.
Bug: The function forgetByQuery unconditionally fails if the deleteMemory SDK call does not return the expected forgotten field, which is an unverified assumption.
Severity: MEDIUM
Suggested Fix
To prevent incorrect failure messages, add a check to verify that the forgotten field exists and is a boolean before evaluating it. For example, check typeof deleted.forgotten === 'boolean' and handle the case where the field is missing, perhaps by assuming success for backward compatibility or logging a warning.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: client.ts#L261-L266
Potential issue: The `forgetByQuery` function relies on the `deleteMemory` method
returning an object with a `forgotten` boolean field. The `deleteMemory` function
directly returns the response from the Supermemory v4 SDK's `memories.forget()` method.
If the SDK's response does not include this `forgotten` field, the check
`!deleted.forgotten` will always evaluate to true. This would cause `forgetByQuery` to
incorrectly report a failure to the user (e.g., `Unable to confirm forgetting: ...`),
even though the underlying memory deletion was successful. This behavior is a regression
and relies on an unverified assumption about the external SDK's API contract.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
Thanks — I checked this against the currently shipped supermemory SDK contract (supermemory@4.15.0), and client.memories.forget() does in fact return a typed MemoryForgetResponse with:
{
id: string;
forgotten: boolean;
}This is defined in the generated SDK types (src/resources/memories.ts and resources/memories.d.ts), where forgotten is explicitly documented as:
Indicates the memory was successfully forgotten
So the deleted.forgotten check here is intentional and aligned with the current SDK/API contract.
The goal of this branch is specifically to avoid false-positive success after fuzzy mis-targeting, not to loosen delete confirmation semantics. If the backend does not confirm forgotten === true, I would prefer to surface that as a failed forget operation rather than reporting success optimistically.
Summary
search/forget/store) back into memoryforgetByQuery()safer by avoiding blind deletion of the first semantic hitforgotten === trueWhy this PR exists
This plugin currently has a few failure modes around
search/forgetthat make memory management feel less trustworthy than it should:forgetByQuery()can delete the wrong memory because it trusts the first semantic search hit too much.Scope of this fix
This PR is intentionally a plugin-layer hardening pass, not a claim that all search/delete ambiguity is fully solved end-to-end.
In particular, this change improves behavior when:
It does not try to redesign or over-assume the backend’s broader memory extraction semantics.
What changed
client.tsforgetByQuery()search a wider candidate set and prefer an exact textual match when availableforgotten === true) before reporting forget successhooks/capture.tsTests
Added focused regression coverage for:
forgetByQuery()target selectionRoot cause addressed here
This patch addresses three concrete plugin-level issues:
Blind first-result deletion
forgetByQuery()effectively searched and deletedresults[0]No exact-match preference
Management-turn recapture
Why the fix is intentionally conservative
I deliberately kept this patch small and local to the plugin.
There may still be broader API-/backend-level behaviors (for example asynchronous extraction or memory text normalization/canonicalization) that affect how quickly or how literally a newly added memory can be searched or forgotten.
This PR does not attempt to solve those larger semantics in the plugin layer.
Instead, it makes the plugin safer and less misleading under the assumptions it can control today.
Testing
bun test bun run check-types bun run lintAll passed locally.