hindsight-langgraph: create_hindsight_tools freezes bank_id at construction time — README Quick Start pattern defeats bank isolation in any multi-tenant deployment

[Fr3ya]

Bug Description

Problem

The hindsight-langgraph integration ships two parallel APIs for connecting agents to a Hindsight memory bank, and the tools API silently ignores per-request configuration while the nodes API correctly honors it.

Tools API — bank_id frozen in closure

hindsight-integrations/langgraph/hindsight_langgraph/tools.py:96-122 (verbatim):

def create_hindsight_tools(*, bank_id: str, client=None, ...):
    ...
    if include_retain:
        @tool
        async def hindsight_retain(content: str) -> str:
            """Store information to long-term memory for later retrieval."""
            try:
                retain_kwargs = {"bank_id": bank_id, "content": content}   # ← closure
                ...
                await resolved_client.aretain(**retain_kwargs)
                return "Memory stored successfully."
            except Exception as e:
                ...

The bank_id parameter is required at construction time and is captured by closure. There is no bank_id_from_config option, no RunnableConfig parameter on the tool function, and no configurable lookup anywhere in the module — grep tools.py for any of bank_id_from_config, RunnableConfig, or configurable returns zero hits in all 291 lines.

Nodes API — bank_id resolved per-request

By contrast, nodes.py:42-118 does the right thing:

def create_recall_node(
    *,
    bank_id: Optional[str] = None,
    ...
    bank_id_from_config: str = "user_id",
    ...
):
    async def recall_node(state, config=None):
        resolved_bank_id = bank_id
        if resolved_bank_id is None and config:
            configurable = config.get("configurable", {})
            resolved_bank_id = configurable.get(bank_id_from_config)
        ...

bank_id is optional; bank_id_from_config="user_id" defaults to reading config["configurable"]["user_id"] per request.

Where the asymmetry bites

The headline Quick Start in hindsight-integrations/langgraph/README.md uses the tools API:

tools = create_hindsight_tools(bank_id="user-123")
agent = create_react_agent(ChatOpenAI(model="gpt-4o"), tools=tools)
result = await agent.ainvoke(
    {"messages": [{"role": "user", "content": "Remember that I prefer dark mode"}]}
)

A natural multi-tenant deployment of this is:

# module scope — agent built once at startup
tools = create_hindsight_tools(bank_id=DEFAULT_BANK)  # placeholder
agent = create_react_agent(ChatOpenAI(...), tools=tools)

@app.post("/chat")
async def chat(req):
    return await agent.ainvoke(
        {"messages": [{"role": "user", "content": req.text}]},
        config={"configurable": {"user_id": req.user_id}},   # ← per-request
    )

This is exactly the pattern the nodes API supports (and create_react_agent itself respects). But the tools — hindsight_retain / hindsight_recall / hindsight_reflect — silently drop config and write/read every user's data into/from the same closure-frozen bank.

CLAUDE.md states the engine's contract:

Bank isolation is strict - no cross-bank data leakage

This is true at the engine layer, but is defeated at the integration layer when a developer uses the Quick Start pattern + serves multiple users.

Steps to Reproduce

Reproducer

Self-contained — no Hindsight server needed (we mock aretain / arecall and check what bank_id they're called with):

import asyncio


class Recorder:
    def __init__(self):
        self.retain_calls = []
        self.recall_calls = []

    async def aretain(self, **kw):
        self.retain_calls.append(kw)
        class _R: success = True
        return _R()

    async def arecall(self, **kw):
        self.recall_calls.append(kw)
        class _R: results = []
        return _R()


def create_hindsight_tools(*, bank_id, client):
    """Verbatim port of tools.py:38-122 (without unrelated config noise)."""
    async def hindsight_retain(content: str) -> str:
        # bank_id from closure; no `config` parameter
        await client.aretain(bank_id=bank_id, content=content)
        return "Memory stored successfully."

    async def hindsight_recall(query: str) -> str:
        await client.arecall(bank_id=bank_id, query=query)
        return "..."

    return [hindsight_retain, hindsight_recall]


class FakeReactAgent:
    """Mimics create_react_agent: holds tools as a closure, ignores config
    at the tool boundary because the tools don't accept it."""
    def __init__(self, tools):
        self.tools = {t.__name__: t for t in tools}

    async def ainvoke(self, payload, config=None):
        msg = payload["messages"][0]["content"]
        await self.tools["hindsight_retain"](content=msg)
        await self.tools["hindsight_recall"](query="user prefs")


recorder = Recorder()
tools = create_hindsight_tools(bank_id="user-123", client=recorder)   # frozen
agent = FakeReactAgent(tools)                                          # singleton


async def main():
    # Two users, two different per-request user_ids
    await agent.ainvoke(
        {"messages": [{"role": "user", "content": "I prefer Italian food."}]},
        config={"configurable": {"user_id": "user-alice"}},
    )
    await agent.ainvoke(
        {"messages": [{"role": "user", "content": "I'm allergic to shellfish."}]},
        config={"configurable": {"user_id": "user-bob"}},
    )
    for c in recorder.retain_calls:
        print(f"retain bank={c['bank_id']!r}  content={c['content']!r}")
    for c in recorder.recall_calls:
        print(f"recall bank={c['bank_id']!r}  query={c['query']!r}")


asyncio.run(main())

Output:

retain bank='user-123'  content='I prefer Italian food.'
retain bank='user-123'  content="I'm allergic to shellfish."
recall bank='user-123'  query='user prefs'
recall bank='user-123'  query='user prefs'

Both Alice's preference and Bob's allergy land in user-123. Subsequent arecall calls from either user return memories pooled from both.

Expected Behavior

Property	Expected	Actual
create_hindsight_tools honors config.configurable.user_id like create_recall_node does	yes (same per-request resolution as the sibling API)	no (closure-frozen bank_id)
Quick Start pattern in hindsight-integrations/langgraph/README.md is safe in a multi-tenant deployment	yes	no (every user hits the same bank)

Actual Behavior

(See above)

Version

No response

LLM Provider

None