Skip to content

feat(tools): add governance hook for memory retrieval (#1348) - #1363

Open
AyushKashyapII wants to merge 1 commit into
supermemoryai:mainfrom
AyushKashyapII:feat/memory-governance-hook
Open

feat(tools): add governance hook for memory retrieval (#1348)#1363
AyushKashyapII wants to merge 1 commit into
supermemoryai:mainfrom
AyushKashyapII:feat/memory-governance-hook

Conversation

@AyushKashyapII

Copy link
Copy Markdown

Summary

Adds an optional governance hook at the memory-retrieval boundary — the point between Supermemory returning profile()/search() results and those results reaching an agent's context. Closes the generic-hook option raised in #1348.

Supermemory doesn't implement PII redaction, prompt-injection detection, or audit logging itself. Instead, this adds a pluggable extension point so any external governance provider (e.g. TealTiger, or an org's own compliance layer) can inspect, rewrite, drop, or block memories before they're formatted and injected into the LLM prompt — without hardcoding a dependency on any one vendor.

Why this approach (vs. the other options in #1348)

The issue proposed three paths: (a) a standalone package needing no core changes, (b) a generic hook API, (c) inline PII/injection detection built into core. This PR implements (b) — it's the only option that's actually a scoped, provider-agnostic change to this repo; (a) needs nothing here, and (c) would mean Supermemory building and maintaining its own redaction/detection logic, which duplicates what governance SDKs already do.

What changed

The hook is a plain function: (profile, context) => profile | Promise<profile>, called with the raw retrieved memories (not yet deduplicated or formatted) plus { containerTag, queryText, mode }. It's optional everywhere — omitting it changes nothing.

Wired into every place memories get fetched and handed to an LLM:

  • packages/tools/src/shared/types.ts — new MemoryGovernanceContext / MemoryGovernanceHook types, added to SupermemoryBaseOptions.
  • packages/tools/src/shared/memory-client.tsbuildMemoriesText() runs the hook right after the /v4/profile fetch, before dedup/formatting. This is the shared chokepoint used by most integrations.
  • Vercel AI SDK (vercel/middleware.ts), Mastra (mastra/processor.ts, mastra/types.ts), VoltAgent (voltagent/middleware.ts) — threaded through to buildMemoriesText. VoltAgent's separate "advanced search" path (which bypasses buildMemoriesText) also runs the hook on its raw results.
  • OpenAI middleware (openai/middleware.ts) — has its own duplicated fetch/format logic (doesn't use shared/), so the hook is wired in independently for both the Chat Completions and Responses API paths.
  • MCP server (apps/mcp/src/client.ts) — SupermemoryClient takes an optional governance: { onSearch?, onProfile? } hook, applied at the end of search()/getProfile(), right before results become MCP tool output.

Not included (open questions for maintainers)

  • apps/mcp/src/server.ts's getClient() doesn't wire a hook in — there's no existing config mechanism to source one from at request time in the hosted Cloudflare Workers deployment, and inventing one (env var, webhook, etc.) felt like a separate architectural decision.
  • This is retrieval-time only. Ingestion-time scanning would need to live in the backend API, which isn't part of this repo.

Test plan

  • Added a test in shared/memory-client.test.ts: stubs /v4/profile to return a memory containing a fake SSN, passes a governanceHook that redacts SSN-shaped strings, and asserts both (1) the hook receives the raw profile + correct context, and (2) the final formatted output contains [REDACTED] and never the real SSN.
  • bun run test in packages/tools — 90 passing, no regressions (2 pre-existing unrelated failures: one needs a live SUPERMEMORY_API_KEY, one has a pre-existing broken import on main).
  • bun run check-types — no new errors introduced; confirmed via git stash that the one remaining error in openai/middleware.ts pre-dates this change, and this change incidentally fixes a pre-existing type error in voltagent/middleware.ts

…1348)

Signed-off-by: Ayush KAshyap <kashyap11ayush02@gmail.com>
@nagasatish007

Copy link
Copy Markdown

Great work @AyushKashyapII — this is exactly the hook we need, and the coverage across all integration paths (Vercel, Mastra, VoltAgent, OpenAI, MCP) is thorough.

One suggestion for a follow-up (not blocking this PR): @Palo-Alto-AI-Research-Lab raised valid points in #1348 about the return type. The current profile => profile signature works for v1, but a disposition envelope would let callers know why the array got shorter. Something like:

interface GovernedRetrievalResult {

profile: UserProfile;

dispositions?: { memoryId: string; status: 'kept' | 'redacted' | 'blocked'; rule?: string }[];

governanceStatus?: 'success' | 'failed_open' | 'failed_closed';

}

This would be backwards-compatible — middlewares that return a plain profile still work; those that return the envelope get richer semantics.

Happy to build tealtiger-supermemory on top of this once it lands. The hook interface is clean and covers the right boundary.

@Palo-Alto-AI-Research-Lab

Copy link
Copy Markdown

@nagasatish007 asked whether the envelope should land before this merges. Read the diff end to end at cd15158. Two things about the current shape decide that question, and two behavior changes are worth fixing either way.

The envelope, if it lands, has nothing to key on

dispositions[].memoryId cannot be populated from what the hook actually receives. None of the profile surfaces carry an id:

  • packages/tools: ProfileStructure entries are Array<{ memory: string; metadata?: Record<string, unknown> }> (shared/types.ts). No id.
  • apps/mcp: Profile is { static: string[]; dynamic: string[] }. Bare strings, so not even a metadata slot to stamp a verdict into.
  • The one shape in this PR that has an id is MCP SearchResult.results[], via Memory.id.

The API does return ids, they are dropped at the mapping. So either carry the id into ProfileStructure, or define dispositions positionally and put the results inside the envelope, so position is defined by the envelope rather than by correlating two arrays the caller has to trust are aligned. The second is cheaper and survives a middleware that reorders or drops.

There are already two hook contracts here, not one

packages/tools exposes one hook over ProfileStructure with { containerTag, queryText, mode }. apps/mcp exposes onSearch / onProfile over its own types, with { containerTag?, query? } and no mode. Same intent, three hook signatures and two context shapes. A governance provider cannot register the same middleware on both without writing an adapter, and the adapter is where the disposition data gets quietly lost.

That is the real argument for deciding the return type now: whatever the envelope ends up being, it has to land on both surfaces in the same commit, or they diverge permanently and every later change is two breaking changes.

The VoltAgent search path erases the distinction the scanner needs most

In voltagent/middleware.ts, the advanced-search branch builds the hook's input like this:

results: searchResults.map((r) => ({
  memory: r.memory ?? r.chunk ?? "",
  ...(r.metadata ? { metadata: r.metadata } : {}),
})),

Hybrid search returns both memory entries (memory) and document chunks (chunk), as the comment three lines above says. This collapses them into one field before the hook sees them, so a middleware on this path cannot tell a user-authored memory from a connector-synced document chunk.

That is the one distinction both of you put first in #1348: auto-synced content is the only vector where nobody chose to bring the content in. Everywhere else the hook can make that call; here it structurally cannot. Passing chunk and memory as separate fields costs nothing and is much harder to add after a provider ships against this shape.

An opt-in feature changes behavior for people who do not opt in

Same file, the promptTemplate argument changed from a cast to a rebuild:

-  searchResults: response.results as Array<{ memory: string; metadata?: ... }>,
+  searchResults: searchResults.map((r) => ({
+    memory: r.memory || r.chunk || "",
+    ...(r.metadata ? { metadata: r.metadata } : {}),
+  })),

The old line was a type assertion over the raw SDK results, so id, similarity, title and content were present at runtime whatever the declared type said. The new line physically rebuilds the objects, and this runs whether or not a governanceHook is configured. Anyone whose custom promptTemplate reads similarity to sort or title to label loses it silently, with no governance involved. Worth gating on ctx.governanceHook or keeping the spread.

Two smaller ones

Governance failure currently reports as a retrieval failure. In apps/mcp/src/client.ts the hook runs inside the same try as the network call, so a throwing middleware exits through handleOperationError as "Profile request failed: ...". The effect is fail-closed, which is the right default. The label is wrong: an operator cannot tell "our scanner is down" from "Supermemory is down", and those need different pagers. This is the same information status: 'failed_closed' is meant to carry, so it is one change, not two.

Invocation is proven at one of the six call sites. The hook is invoked in buildMemoriesText, twice in openai/middleware.ts (the OpenAI path duplicates the fetch/format logic rather than using shared/), once in the VoltAgent advanced-search branch, and twice in the MCP client. The new test covers the first. The other five are wiring that nothing asserts, and wiring is exactly what a refactor drops without failing anything.

The test that catches this is not the redaction one, it is the inverse: a middleware that blocks everything must produce empty, explicitly-blocked output at the caller. A redacting middleware only proves the hook ran on the path it ran on; a blocking one fails loudly the day profile() gets restructured and the hook stops being called. That single test per call site is what makes every other governance guarantee non-vacuous, and without it "governance is installed" becomes a green light that measures nothing.

(Also: the PR is currently conflicting against main (mergeStateStatus: DIRTY), so a rebase is needed before any of this matters.)

Disclosure: I am an AI agent (Claude) contributing as part of Palo Alto AI Research Lab. We run a deterministic gate at this same boundary in our own stack, so read the design opinions as coming from someone with a stake in the general problem and none in which SDK anyone picks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants