test(recall): pin the omitted-filter identity binding

[Eldad-Caura]

Adds the one test #1268 should have had, from @zznate's #995.

The gap

#1268 fixed /recall's identity resolution and shipped five tests. Every one of them names an identity explicitly — a spoofed filter_agent_id, a spoofed caller_agent_id, an agent naming itself, a tenant key naming a peer, caller_agent_id honoured. So the default path went unpinned: no filter, no caller id, just an ordinary recall.

That is also the worse of the two consequences in the original report (#994), because it needs no crafted request. Pre-fix, caller_agent_id=body.filter_agent_id meant an omitted filter passed None — the tenant-wide visibility a tenant credential gets — and the trust<2 fleet forcing sat inside if body.filter_agent_id:, so it did not run either. A trust-1 agent issuing the most ordinary possible recall read across fleets.

The behaviour is correct on main today. It just was not held there by anything.

Confirmed, not assumed

Reverting only the /recall callsite to the pre-fix derivation:

AssertionError: an omitted filter did not bind to the authenticated agent:
  caller_agent_id=None, expected 'agent-a-5f54f57c'

None is precisely the tenant-wide identity the finding described.

Asserted on arguments, not on results

The test captures what reaches search_memories rather than comparing result sets. A results-parity assertion cannot separate "bound to the caller" from "happened to return the same rows" — an unbound read coincides with a bound one whenever the caller has nothing hidden from it, and then the assertion passes while the identity is still wrong.

The patch seam is the interesting part

/recall re-imports the function inside the handler:

from core_api.services.memory_service import search_memories

So it resolves the name at call time from the service module and never sees a patch applied to the route module's own binding — which is the one /search uses. Patching the route seam silently no-ops: the real search runs and the assertions then read whatever that call left behind.

My first attempt did exactly that and passed for the wrong reason until the removal probe exposed it. #995 patched the memory_service seam from the start. Their choice was right where mine was wrong, and there is now a comment on the test saying why, so the next person does not re-learn it.

Attribution

The case and the argument-level assertion are @zznate's, from #995 — their fix for their own report in #994. #1268 landed the shared resolver first and closed a vector #995 did not (a spoofed caller_agent_id, which /recall accepts because it parses SearchRequest), which is why #995 is superseded rather than merged. This test is not.

Verification

• Full root suite: 6049 passed, 5 skipped, 1 xfailed, 0 failed, run as .venv/bin/python -m pytest. The diff is one new test function.
• ruff check and ruff format --check at CI's tests/ scope — clean.
• legacy_name_ratchet.py → No new lines. · do_not_touch_sentinel.py → All 39 protected strings survive. Both after git add.
• Branched fresh from origin/main at 7c94e1db.
• Test-only, so claude-review's source-file filter will skip this PR by design.

🤖 Generated with Claude Code

[github-actions]

Claude Code Review — skipped: no source files to review (only tests/docs/config)

[Eldad-Caura]

@erni-a ready for human review at head ca0e276c. Small — one test function.

Worth flagging: this PR has had no automated review. claude-review skipped it by design ("no source files to review (only tests/docs/config)"), which is correct for a test-only change but means a human read is the only review it gets.

What it adds: the omitted-filter case for /recall's identity binding. #1268 fixed that resolution and shipped five tests, every one of which names an identity explicitly — so the default path (no filter_agent_id, no caller_agent_id) went unpinned. That is the case needing no crafted request, and it is the consequence the original reporter flagged second: pre-fix an omitted filter passed caller_agent_id=None, the tenant-wide visibility a tenant credential gets, and the trust<2 fleet forcing did not run either.

The behaviour on main is correct. Nothing was holding it there.

Confirmed by probe — reverting only the /recall callsite gives caller_agent_id=None, expected 'agent-a-…'.

Two details a reviewer should not have to rediscover, both called out in comments on the test:

1. The patch target is memory_service.search_memories, not the route module's binding. /recall re-imports the function inside the handler, so it resolves at call time from the service module and never sees a patch on the route's own name — the one /search uses. Patching the route seam silently no-ops and the real search runs. My first draft did that and passed for the wrong reason.
2. It asserts on the arguments reaching search_memories rather than on result-set parity, which cannot separate "bound to the caller" from "happened to return the same rows".

Attribution: the case and the argument-level assertion are @zznate's, from #995 (their fix for their report in #994). #995 is closed as superseded — #1268 covered a vector it did not — but this test was not superseded, and it is theirs.

Full root suite 6049 passed / 0 failed; ruff check and format at CI's tests/ scope, ratchet and sentinel all clean after git add. Branched from origin/main at 7c94e1db. Not merging — that's Eldad's.