Skip to content

fix: harden search/forget behavior and skip capture for memory-management turns #25

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
pascalkienast wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: harden search/forget behavior and skip capture for memory-management turns #25

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 94 additions & 7 deletions client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,81 @@ function limitText(text: string, max: number): string {
return text.length > max ? `${text.slice(0, max)}…` : text
}

export function normalizeMemoryText(text: string | undefined): string {
return sanitizeContent(text ?? "")
.trim()
.replace(/\s+/g, " ")
.toLowerCase()
}

function getQueryTokens(normalizedQuery: string): string[] {
if (!normalizedQuery) return []
return normalizedQuery.split(/\s+/).filter(Boolean)
}

function getTokenCoverage(
normalizedText: string,
queryTokens: string[],
): number {
if (queryTokens.length === 0) return 0
const matched = queryTokens.filter((token) =>
normalizedText.includes(token),
).length
return matched / queryTokens.length
}

export function findExactContentMatch(
results: SearchResult[],
query: string,
): SearchResult | undefined {
const normalizedQuery = normalizeMemoryText(query)
if (!normalizedQuery) return undefined

return results.find(
(result) =>
normalizeMemoryText(result.content || result.memory) === normalizedQuery,
)
}

export function rerankSearchResults(
query: string,
results: SearchResult[],
): SearchResult[] {
const normalizedQuery = normalizeMemoryText(query)
if (!normalizedQuery || results.length <= 1) return results

const queryTokens = getQueryTokens(normalizedQuery)

return [...results]
.map((result, index) => {
const normalizedContent = normalizeMemoryText(
result.content || result.memory,
)
return {
index,
result,
exact: normalizedContent === normalizedQuery ? 1 : 0,
contains: normalizedContent.includes(normalizedQuery) ? 1 : 0,
tokenCoverage: getTokenCoverage(normalizedContent, queryTokens),
similarity: result.similarity ?? 0,
contentLength: normalizedContent.length || Number.MAX_SAFE_INTEGER,
}
})
.sort((a, b) => {
if (b.exact !== a.exact) return b.exact - a.exact
if (b.contains !== a.contains) return b.contains - a.contains
if (b.tokenCoverage !== a.tokenCoverage) {
return b.tokenCoverage - a.tokenCoverage
}
if (b.similarity !== a.similarity) return b.similarity - a.similarity
if (a.contentLength !== b.contentLength) {
return a.contentLength - b.contentLength
}
return a.index - b.index
})
.map(({ result }) => result)
}

export class SupermemoryClient {
private client: Supermemory
private containerTag: string
Expand Down Expand Up @@ -90,27 +165,31 @@ export class SupermemoryClient {
limit = 5,
containerTag?: string,
): Promise<SearchResult[]> {
const cleanedQuery = sanitizeContent(query)
const tag = containerTag ?? this.containerTag

log.debugRequest("search.memories", {
query,
query: cleanedQuery,
limit,
containerTag: tag,
})

const response = await this.client.search.memories({
q: query,
q: cleanedQuery,
containerTag: tag,
limit,
rerank: true,
rewriteQuery: false,
})

const results: SearchResult[] = (response.results ?? []).map((r) => ({
const rawResults: SearchResult[] = (response.results ?? []).map((r) => ({
id: r.id,
content: r.memory ?? "",
memory: r.memory,
similarity: r.similarity,
metadata: r.metadata ?? undefined,
}))
const results = rerankSearchResults(cleanedQuery, rawResults)

log.debugResponse("search.memories", { count: results.length })
return results
Expand Down Expand Up @@ -168,17 +247,25 @@ export class SupermemoryClient {
query: string,
containerTag?: string,
): Promise<{ success: boolean; message: string }> {
log.debugRequest("forgetByQuery", { query, containerTag })
const cleanedQuery = sanitizeContent(query)
log.debugRequest("forgetByQuery", { query: cleanedQuery, containerTag })

const results = await this.search(query, 5, containerTag)
const results = await this.search(cleanedQuery, 10, containerTag)
if (results.length === 0) {
return { success: false, message: "No matching memory found to forget." }
}

const target = results[0]
await this.deleteMemory(target.id, containerTag)
const target = findExactContentMatch(results, cleanedQuery) ?? results[0]
const deleted = await this.deleteMemory(target.id, containerTag)

const preview = limitText(target.content || target.memory || "", 100)
if (!deleted.forgotten) {
return {
success: false,
message: `Unable to confirm forgetting: "${preview}"`,
}
Comment on lines 261 to +266
Copy link

@sentry sentry bot Mar 14, 2026

Choose a reason for hiding this comment

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

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.

Copy link
Author

@pascalkienast pascalkienast Mar 14, 2026

Choose a reason for hiding this comment

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

Thanks — I checked this against the currently shipped supermemory SDK contract ([email protected]), 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.

}

return { success: true, message: `Forgot: "${preview}"` }
}

Expand Down
114 changes: 99 additions & 15 deletions hooks/capture.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,17 @@ import { log } from "../logger.ts"
import { buildDocumentId } from "../memory.ts"

const SKIPPED_PROVIDERS = ["exec-event", "cron-event", "heartbeat"]
const MEMORY_TOOL_PREFIX = "supermemory_"
const MEMORY_COMMAND_PREFIXES = ["/remember", "/recall"]
const MEMORY_TOOL_RESPONSE_PATTERNS = [
/^Stored:\s*"/i,
/^Forgot:\s*"/i,
/^Found \d+ memories:/i,
/^No relevant memories found\.?$/i,
/^No matching memory found to forget\.?$/i,
/^Memory forgotten\.?$/i,
/^Provide a query or memoryId to forget\.?$/i,
]

function getLastTurn(messages: unknown[]): unknown[] {
let lastUserIdx = -1
Expand All @@ -21,6 +32,89 @@ function getLastTurn(messages: unknown[]): unknown[] {
return lastUserIdx >= 0 ? messages.slice(lastUserIdx) : messages
}

function collectTextParts(content: unknown): string[] {
const parts: string[] = []

if (typeof content === "string") {
parts.push(content)
return parts
}

if (!Array.isArray(content)) return parts

for (const block of content) {
if (!block || typeof block !== "object") continue
const b = block as Record<string, unknown>
if (b.type === "text" && typeof b.text === "string") {
parts.push(b.text)
}
}

return parts
}

function messageReferencesSupermemoryTool(
msgObj: Record<string, unknown>,
): boolean {
for (const key of ["name", "toolName"]) {
if (
typeof msgObj[key] === "string" &&
(msgObj[key] as string).startsWith(MEMORY_TOOL_PREFIX)
) {
return true
}
}

const content = msgObj.content
if (!Array.isArray(content)) return false

for (const block of content) {
if (!block || typeof block !== "object") continue
const b = block as Record<string, unknown>
if (typeof b.name === "string" && b.name.startsWith(MEMORY_TOOL_PREFIX)) {
return true
}
if (
typeof b.toolName === "string" &&
b.toolName.startsWith(MEMORY_TOOL_PREFIX)
) {
return true
}
}

return false
}

export function isSupermemoryManagementTurn(messages: unknown[]): boolean {
for (const msg of messages) {
if (!msg || typeof msg !== "object") continue
const msgObj = msg as Record<string, unknown>

if (messageReferencesSupermemoryTool(msgObj)) {
return true
}

for (const text of collectTextParts(msgObj.content)) {
const trimmed = text.trim()
const lower = trimmed.toLowerCase()
if (
MEMORY_COMMAND_PREFIXES.some(
(prefix) => lower === prefix || lower.startsWith(`${prefix} `),
)
) {
return true
}
if (
MEMORY_TOOL_RESPONSE_PATTERNS.some((pattern) => pattern.test(trimmed))
) {
return true
}
}
}

return false
}

export function buildCaptureHandler(
client: SupermemoryClient,
cfg: SupermemoryConfig,
Expand All @@ -46,6 +140,10 @@ export function buildCaptureHandler(
return

const lastTurn = getLastTurn(event.messages)
if (isSupermemoryManagementTurn(lastTurn)) {
log.debug("capture: skipping supermemory management turn")
return
}

const texts: string[] = []
for (const msg of lastTurn) {
Expand All @@ -54,21 +152,7 @@ export function buildCaptureHandler(
const role = msgObj.role
if (role !== "user" && role !== "assistant") continue

const content = msgObj.content

const parts: string[] = []

if (typeof content === "string") {
parts.push(content)
} else if (Array.isArray(content)) {
for (const block of content) {
if (!block || typeof block !== "object") continue
const b = block as Record<string, unknown>
if (b.type === "text" && typeof b.text === "string") {
parts.push(b.text)
}
}
}
const parts = collectTextParts(msgObj.content)

if (parts.length > 0) {
texts.push(`[role: ${role}]\n${parts.join("\n")}\n[${role}:end]`)
Expand Down
Loading