CAURA-723 feat(search): name the agent filter when it is why the resu… by eyal-bl · Pull Request #1434 · caura-ai/caura

eyal-bl · 2026-09-09T12:59:53Z

…lt is empty

A tenant-scoped caller that passes a wrong filter_agent_id gets HTTP 200 · items [] · warnings null — byte-identical to a correct id that simply has nothing relevant to say. Measured, one memory written by agent-real:

filter_agent_id = "agent-real"  -> 200  items=1
filter_agent_id = "agnet-real"  -> 200  items=0  warnings=None

The filter is a SQL WHERE memories.agent_id = ?, so a typo matches no rows and the query returns nothing exactly as an empty query would. Nothing in the response separates them, and it fails in the safe-looking direction — an empty list reads as "no data yet", the most ordinary thing a memory product can say, so nobody investigates. It cost a 589-query benchmark run against an empty store.

Scope: this is a tenant-scoped-caller bug only

An agent-scoped credential carries a verified X-Agent-ID, and enforce_self_agent 403s it for naming any agent but itself — measured, for a typo and for a peer id. So the silent path needs a credential with no agent identity, asserting the id in the body.

That is not a corner: it is the dashboard's hand-typed filter_agent_id text box (tenant-scoped by its own comment, "no X-Agent-ID"), the documented caura_recall parameter on the public for-agents page, every benchmark harness we own, and the multi-user backend shape where one key serves many end users. Gating on the asserted identity keeps the probe off the path where the condition is unreachable.

Which table decides what, and why not the obvious one

The natural implementation — look the id up in agents, and if absent report "no such agent" and skip — is wrong in the direction that loses data. DELETE /agents/{id} says so itself: "Delete an agent. Memories written by this agent are NOT deleted." The rows stay live and searchable with no agent row, and rows predating agent tracking were never registered at all. So:

has_memories (the memories table) decides whether to skip. It is the only fact that can prove the search would return nothing.
agent_registered (the agents table) decides only the wording, where being wrong costs a slightly-off message.

Four states, three codes:

memories  agents    ->  outcome
none      absent        skip · filter_agent_unknown
none      present       skip · filter_agent_empty
some      absent        RUN THE SEARCH · filter_agent_deregistered
some      present       run the search, no warning

Row three is the regression guard: the memories outlive the agent row, so they must still be returned.

Ordering, and a hole the ordering first created

The probe is computed immediately after identity resolution and BEFORE the route's get_or_create_agent, which registers the asserted id — a typo included. Probing after it would find an agents row the read itself had just created, and every typo would report as registered.

The first draft also RETURNED there, which skipped enforce_fleet_read_many and the usage metering: a caller naming a fleet it has no rights to got a 200 with a warning instead of a 403, and the warning disclosed whether an agent id exists.
test_c27_strict_fleet_scoping and test_h05_multi_fleet_read_gate caught it. The short-circuit now sits after every gate; the saving it exists for — the embedding call and the scored search (plus the recall model call on /recall) — is still entirely ahead of that point.

Cost

One round trip answering both halves, only on the asserted-identity path with a filter set. Two LIMIT 1 index probes:
ix_memories_tenant_agent and uq_agents_tenant_agent. A search that returns results pays that and nothing else; a misconfigured one is strictly cheaper than today, trading an embedding API call and a vector scan for two index hits.

read=True deliberately: the search it explains reads the replica, so answering from the same replica keeps the probe's story consistent with the result rather than reporting rows the search could not see.

The probe never raises. A failure degrades to today's behaviour — an unexplained result — because a diagnostic hint must not be able to fail a working search.

Ratchets moved, both deliberately

test_c27_strict_fleet_scoping pins two things, and the probe is a fifth fleet-scoped read:

it must go through _fleet_scope_clause, never a hand-rolled fleet_id.in_(fleet_ids) — "exactly how A54 leaked". It does.
the helper's call-site count moves 5 -> 6, with the reasoning recorded at the constant.

The probe calls the helper with strict=False on purpose. Non-strict is a SUPERSET (it also admits tenant-shared null-fleet rows and scope_org), so the probe stays at least as permissive as the search it explains and can never skip a search that would have returned rows. Threading the tenant's real strict_fleet_scoping would tighten it and buy exactly that risk.

`/recall` too

It parses the same SearchRequest and had the identical silent empty. Leaving it out is how the two routes came to disagree about what these fields mean — the reason _resolve_read_identity is shared at all. Its response gains warnings, seeded with any scope warning and then extended by the pipeline, so the field means what /search's does rather than carrying only this one code.

Known limitation, pre-existing

The read path registers the asserted id via get_or_create_agent, so a repeated typo is registered by the first call and the second reports filter_agent_empty rather than filter_agent_unknown. Both still say the filter matched nothing, and the wording is advisory. Not introduced here and not fixed here: the clean answers are to stop registering on reads, or agent soft-delete (a deleted_at plus making uq_agents_tenant_agent partial, or re-registration breaks) — both larger changes than a warning warrants.

Verification

tests/test_caura723_agent_scope_warning.py — 9 cases: the four states, the fleet-narrowed probe, /recall, the agent-scoped 403 left unchanged, no-filter and good-filter silence, and a failing probe leaving search working. 5 fail without the source change; the other 4 are no-regression guards that must hold in both directions.
Targeted sweep post-rebase: 111 passed (test_c27_strict_fleet_scoping, test_h05_multi_fleet_read_gate, test_search_recall_tracked_flag, test_search_caller_identity, test_c4_recall_items_alias, test_mcp_recall, test_route_authz_gaps).
Full tests/: 39 failures, byte-identical to main's set on this machine (pre-existing FTS / relation-weight / request-observation env failures). core-storage-api/tests/: 369 passed, same 2 pre-existing CORS failures.
ruff check and format clean; mypy identical to main on every changed file. do_not_touch_sentinel all 35 survive, tenant_scope_gate exit 0, legacy_name_ratchet reports one exempt legacy-name-ok line: the test patches settings.memclaw_api_key because that is the dual-read field auth Path 2 reads — patching caura_api_key is inert, since the two collapse at validation time.

No schema changes.

Summary

Related Issue

Type of Change

Bug fix (non-breaking change that fixes an issue)
New feature (non-breaking change that adds functionality)
Breaking change (fix or feature that changes existing behavior)
Documentation update
Refactor / internal cleanup
Other:

How Has This Been Tested?

Checklist

I have read CONTRIBUTING.md
I have added tests that cover my changes (or explained why none are needed)
ruff check and ruff format --check pass
mypy passes
pytest passes locally
I have updated relevant documentation (README, docs, etc.)
I have updated CHANGELOG.md under the Unreleased section (if user-facing)

Additional Notes

github-actions · 2026-09-09T13:00:03Z

Claude Code Review — skipped: PR author 'eyal-bl' is not a public member of the 'caura-ai' org

eyal-bl · 2026-09-09T13:00:08Z

@claude

github-actions · 2026-09-09T13:00:24Z

Claude Code Review — skipped: PR author 'eyal-bl' is not a public member of the 'caura-ai' org

github-actions · 2026-09-09T13:04:25Z

🤖 Review by Claude Code

Summary

This PR adds an "agent scope probe" that explains why an agent-filtered /search or /recall call returns empty (typo'd agent id vs. a real agent with no memories vs. a deregistered agent whose memories still exist). The core logic in agent_scope.py and postgres_service.py is well-reasoned and defensively coded (fails open, mirrors the search's own tenant/fleet scoping, biases toward running the search rather than skipping it). One real issue stands out around the added latency this introduces to the hot path.

Medium/Low Issues

Probe adds an unconditional extra round-trip to every filtered search/recall, contradicting its own docstring

Severity: Medium
File: core-api/src/core_api/clients/storage_client.py:2593-2603, core-api/src/core_api/routes/memories.py:1900-1912, core-api/src/core_api/routes/memories.py:2235-2247
Problem: The agent_scope_probe docstring claims the probe "runs only when a search is about to return nothing," but the actual call sites in _search_inner and recall_endpoint invoke probe_asserted_agent_scope unconditionally whenever an agent id is present (asserted or filtered) — before the real search even runs — adding one extra network round trip (with two SQL queries on the storage side) to every agent-scoped search/recall, including the common successful case where nothing is wrong.

🤖 Claude Code Prompt

In core-api/src/core_api/clients/storage_client.py around lines 2593-2603, the
docstring for agent_scope_probe states the probe "runs only when a search is
about to return nothing," but this is inaccurate: the call sites in
core-api/src/core_api/routes/memories.py (_search_inner around lines
1900-1912, and recall_endpoint around lines 2235-2247) invoke
probe_asserted_agent_scope unconditionally whenever an agent id is present,
before the actual search query executes, regardless of whether the search
would return results. This means every agent-filtered search/recall pays an
extra HTTP round trip + two SQL queries even in the fully successful case.

Either:
1) Correct the docstring in storage_client.py to accurately describe that the
   probe always runs for agent-scoped requests (removing the misleading
   "only when about to return nothing" claim), so future readers don't
   assume this call is free on the success path, or
2) If the added latency on every filtered request is a real concern, restructure
   so the probe is only invoked after the primary search executes and returns
   zero rows (this would require moving get_or_create_agent's registration
   after the probe, or reordering so the probe only fires on an empty result,
   while still avoiding the self-fulfilling-registration problem documented in
   core_api/services/agent_scope.py).
At minimum, fix the misleading docstring so it matches actual behavior, and
consider benchmarking the added latency on production-representative
agent-filtered search/recall traffic.

Reviewed by claude-sonnet-5 · cost $0.6470149999999999

github-actions · 2026-09-09T13:47:57Z

Claude Code Review — skipped: PR author 'eyal-bl' is not a public member of the 'caura-ai' org

…lt is empty A tenant-scoped caller that passes a wrong `filter_agent_id` gets `HTTP 200 · items [] · warnings null` — byte-identical to a correct id that simply has nothing relevant to say. Measured, one memory written by `agent-real`: filter_agent_id = "agent-real" -> 200 items=1 filter_agent_id = "agnet-real" -> 200 items=0 warnings=None The filter is a SQL `WHERE memories.agent_id = ?`, so a typo matches no rows and the query returns nothing exactly as an empty query would. It fails in the safe-looking direction — an empty list reads as "no data yet", the most ordinary thing a memory product can say, so nobody investigates. It cost a 589-query benchmark run against an empty store. ## Scope: a tenant-scoped-caller bug only An agent-scoped credential carries a verified `X-Agent-ID`, and `enforce_self_agent` **403s** it for naming any agent but itself — measured, for a typo and for a peer id. So the silent path needs a credential with no agent identity, asserting the id in the body. Not a corner: it is the dashboard's hand-typed `filter_agent_id` text box (tenant-scoped by its own comment, "no X-Agent-ID"), the documented `caura_recall` parameter on the public for-agents page, every benchmark harness we own, and the multi-user backend shape where one key serves many end users. ## When it runs, and what it costs Only after the search, and the storage probe only when the search came back EMPTY. **A successful agent-filtered search pays nothing** — no extra round trip, no extra query. Pinned by `test_a_successful_search_makes_no_probe_call`, which counts calls rather than trusting the reading. This is the second attempt, after review of #1434. The first probed BEFORE the search to skip the embedding call on a misconfigured request — but that saving lands on requests already broken, which are rare, while the cost lands on every healthy one, which is the norm. Wrong way round on expected cost. The review also caught that the docstring claimed the probe "runs only when a search is about to return nothing", which was false of that ordering; it is true of this one. The empty path is now one query, not two: `include_agent_registered` lets the probe skip the `agents` lookup, because the route already made it (see below). ## Which fact decides what, and why not the obvious one The natural implementation — look the id up in `agents`, and if absent report "no such agent" — is wrong in the direction that misleads. `DELETE /agents/{id}` says so itself: *"Delete an agent. Memories written by this agent are NOT deleted."* The rows stay live and searchable with no agent row, and rows predating agent tracking were never registered at all. A missing agent row means "deregistered or never registered", never "nothing to find". So `has_memories` establishes there was nothing to find, and `agent_preexisted` only chooses the wording: returned rows + agent gone -> filter_agent_deregistered (free) empty + has_memories + gone -> filter_agent_deregistered empty + no memories + known -> filter_agent_empty empty + no memories + unknown -> filter_agent_unknown anything else -> silence `agent_preexisted` comes from the route's own `get_or_create_agent`, via a new `registration_ctx` out-dict in the same shape as `diagnostic_ctx` / `warnings_ctx` / `recall_ctx`. That call already does the `agents` lookup, so it is free — and it *has* to come from there, because by the time the search has run the row exists whether or not it did beforehand, and asking afterwards would report every typo as a registered agent. An out-dict rather than a changed return type so the other seven callers stay untouched. Known boundary: an admin credential skips `get_or_create_agent` (`if auth.tenant_id:`), so it has no free pre-existence signal and is not told an agent is deregistered on a NON-empty result. Buying that would put a query back on the success path. Documented on the `tenant_scoped` fixture. ## Ratchets `test_c27_strict_fleet_scoping` pins two things, and the probe is a fifth fleet-scoped read: * it must go through `_fleet_scope_clause`, never a hand-rolled `fleet_id.in_(fleet_ids)` — "exactly how A54 leaked". It does. * the helper's call-site count moves 5 -> 6, with the reasoning recorded at the constant. `strict=False` on purpose: non-strict is a SUPERSET (it also admits tenant-shared null-fleet rows and `scope_org`), so the probe stays at least as permissive as the search it explains and can never report "no memories" for rows the search could see. An earlier draft short-circuited ahead of `enforce_fleet_read_many` and the usage metering — a caller naming a fleet it has no rights to got a 200 with a warning instead of a 403, and the warning disclosed whether an agent id exists. `test_c27_strict_fleet_scoping` and `test_h05_multi_fleet_read_gate` caught it. That whole class is gone with the short-circuit: the search now always runs. ## `/recall` too It parses the same `SearchRequest` and had the identical silent empty. Leaving it out is how the two routes came to disagree about what these fields mean — the reason `_resolve_read_identity` is shared at all. Its response gains `warnings`, seeded from the probe and then extended by the pipeline, so the field means what `/search`'s does rather than carrying only this one code. ## Known limitation, pre-existing The read path registers the asserted id via `get_or_create_agent`, so a repeated typo is registered by the first call and the second reports `filter_agent_empty` rather than `filter_agent_unknown`. Both still say the filter matched nothing, and the wording is advisory. Not introduced here and not fixed here: the clean answers are to stop registering on reads, or agent soft-delete (a `deleted_at` plus making `uq_agents_tenant_agent` partial, or re-registration breaks) — both larger than a warning warrants. ## Verification - `tests/test_caura723_agent_scope_warning.py` — 10 cases: the four states, the fleet-narrowed probe, `/recall`, the agent-scoped 403 left unchanged, no-filter and good-filter silence, a failing probe leaving search working, and the no-cost-on-success contract. - Targeted sweep post-rebase: 110 passed (`test_c27_strict_fleet_scoping`, `test_h05_multi_fleet_read_gate`, `test_route_authz_gaps`, `test_search_recall_tracked_flag`, `test_c4_recall_items_alias`, `test_mcp_recall`, `test_agent_admin_authz`). - Full `tests/`: 39 failures, byte-identical to `main`'s set on this machine (pre-existing FTS / relation-weight / request-observation env failures). `core-storage-api/tests/`: 369 passed, same 2 pre-existing CORS failures. - ruff check and format clean; mypy identical to `main` on every changed file. `do_not_touch_sentinel` all 35 survive, `tenant_scope_gate` exit 0, `legacy_name_ratchet` **"No new lines."** — the test reaches the dual-read Settings field through `tests/_legacy_contracts`' `LEGACY_API_KEY_FIELD` rather than minting the literal, in keeping with #1436-#1438. No schema changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Eyal Blyachman <eyal.b@caura.ai>

github-actions · 2026-09-09T14:16:02Z

Claude Code Review — skipped: PR author 'eyal-bl' is not a public member of the 'caura-ai' org

github-actions · 2026-09-09T16:08:54Z

Claude Code Review — skipped: PR author 'eyal-bl' is not a public member of the 'caura-ai' org

eyal-bl · 2026-09-09T16:09:04Z

@claude

github-actions · 2026-09-09T16:14:46Z

🤖 Review by Claude Code

Summary

This PR adds a diagnostic "why did my agent-filtered search return nothing" warning to /search and /recall, wired through a new low-cost storage-side probe. The implementation is well thought through for the single-field case (only filter_agent_id set), with genuine care taken to keep the healthy path free of extra cost and to avoid the "not-in-agents-table means no memories" trap. However, the warning is keyed off the resolved identity (eff_agent_id) rather than the actual SQL filter parameter (filter_agent_id) that the feature's own docstring uses as its motivating example, which creates a real gap when both fields are supplied with different values.

Medium/Low Issues

Agent-scope warning is keyed on the resolved identity, not the actual `filter_agent_id`, so it can misidentify or miss the real cause of an empty result

Severity: Medium
File: core-api/src/core_api/routes/memories.py (both explain_agent_scope(...) call sites in _search_inner and recall_endpoint); core-api/src/core_api/services/agent_scope.py:96-105
Problem: asserted_agent_id is derived from eff_agent_id (which, per _resolve_read_identity, prefers body.caller_agent_id over body.filter_agent_id), but the actual SQL restriction that can silently return an empty result is always body.filter_agent_id, passed to search_memories as a separate, untouched parameter — so when a caller sets both fields to different values, the probe/warning explains the wrong id.

🤖 Claude Code Prompt

In core-api/src/core_api/routes/memories.py, both call sites that invoke
`explain_agent_scope(...)` (inside `_search_inner`, around the
`search_warnings.extend(...)` block, and inside `recall_endpoint`, around the
`recall_warnings.extend(...)` block) pass `asserted_agent_id=eff_agent_id if
not auth.agent_id else None`.

`eff_agent_id` comes from `_resolve_read_identity`, which resolves as
`auth.agent_id or (AgentIdentity(body.caller_agent_id or body.filter_agent_id))`
— i.e. it prefers `caller_agent_id` over `filter_agent_id` when both are set.
But the literal SQL restriction that actually determines which rows can match
(and therefore whether the result can be silently empty) is always
`body.filter_agent_id`, passed to `search_memories` as its own, separate
`filter_agent_id=body.filter_agent_id` argument — never through `eff_agent_id`.

Concretely: if a tenant-scoped caller sets `caller_agent_id="agent-real"` (a
valid, existing identity, used only for scope_agent visibility) together with
`filter_agent_id="agnet-real"` (a typo, the actual literal filter causing the
empty result), `eff_agent_id` resolves to `"agent-real"` (caller_agent_id wins
by precedence). `explain_agent_scope` then probes/reports on `"agent-real"`
(which has memories and is registered), producing no warning at all — even
though the empty result is caused by the typo in `filter_agent_id`, which is
exactly the scenario CAURA-723's own docstring (in
core-api/src/core_api/services/agent_scope.py) cites as the motivating defect.

Fix by having `explain_agent_scope` reason about `body.filter_agent_id`
whenever it is set (since that is what actually restricts the query), falling
back to the identity-based `eff_agent_id` only when `filter_agent_id` is
absent (the `caller_agent_id`-only case). This likely means passing both
`body.filter_agent_id` and the resolved identity into `explain_agent_scope`
(or computing `asserted_agent_id` at the call site as
`body.filter_agent_id or (eff_agent_id if not auth.agent_id else None)`) so
the probe/warning always targets the id that can actually cause the empty
result, and add a test covering the case where `caller_agent_id` and
`filter_agent_id` are both set to different values (one real, one typo`d).

Reviewed by claude-sonnet-5 · cost $1.5293145000000001

…lt is empty A tenant-scoped caller that passes a wrong `filter_agent_id` gets `HTTP 200 · items [] · warnings null` — byte-identical to a correct id that simply has nothing relevant to say. Measured, one memory written by `agent-real`: filter_agent_id = "agent-real" -> 200 items=1 filter_agent_id = "agnet-real" -> 200 items=0 warnings=None The filter is a SQL `WHERE memories.agent_id = ?`, so a typo matches no rows and the query returns nothing exactly as an empty query would. It fails in the safe-looking direction — an empty list reads as "no data yet", the most ordinary thing a memory product can say, so nobody investigates. It cost a 589-query benchmark run against an empty store. ## Scope: a tenant-scoped-caller bug only An agent-scoped credential carries a verified `X-Agent-ID`, and `enforce_self_agent` **403s** it for naming any agent but itself — measured, for a typo and for a peer id. So the silent path needs a credential with no agent identity, asserting the id in the body. Not a corner: it is the dashboard's hand-typed `filter_agent_id` text box (tenant-scoped by its own comment, "no X-Agent-ID"), the documented `caura_recall` parameter on the public for-agents page, every benchmark harness we own, and the multi-user backend shape where one key serves many end users. ## When it runs, and what it costs Only after the search, and the storage probe only when the search came back EMPTY. **A successful agent-filtered search pays nothing** — no extra round trip, no extra query. Pinned by `test_a_successful_search_makes_no_probe_call`, which counts calls rather than trusting the reading. This is the second attempt, after review of #1434. The first probed BEFORE the search to skip the embedding call on a misconfigured request — but that saving lands on requests already broken, which are rare, while the cost lands on every healthy one, which is the norm. Wrong way round on expected cost. The review also caught that the docstring claimed the probe "runs only when a search is about to return nothing", which was false of that ordering; it is true of this one. The empty path is now one query, not two: `include_agent_registered` lets the probe skip the `agents` lookup, because the route already made it (see below). ## Which fact decides what, and why not the obvious one The natural implementation — look the id up in `agents`, and if absent report "no such agent" — is wrong in the direction that misleads. `DELETE /agents/{id}` says so itself: *"Delete an agent. Memories written by this agent are NOT deleted."* The rows stay live and searchable with no agent row, and rows predating agent tracking were never registered at all. A missing agent row means "deregistered or never registered", never "nothing to find". So `has_memories` establishes there was nothing to find, and `agent_preexisted` only chooses the wording: returned rows + agent gone -> filter_agent_deregistered (free) empty + has_memories + gone -> filter_agent_deregistered empty + no memories + known -> filter_agent_empty empty + no memories + unknown -> filter_agent_unknown anything else -> silence `agent_preexisted` comes from the route's own `get_or_create_agent`, via a new `registration_ctx` out-dict in the same shape as `diagnostic_ctx` / `warnings_ctx` / `recall_ctx`. That call already does the `agents` lookup, so it is free — and it *has* to come from there, because by the time the search has run the row exists whether or not it did beforehand, and asking afterwards would report every typo as a registered agent. An out-dict rather than a changed return type so the other seven callers stay untouched. Known boundary: an admin credential skips `get_or_create_agent` (`if auth.tenant_id:`), so it has no free pre-existence signal and is not told an agent is deregistered on a NON-empty result. Buying that would put a query back on the success path. Documented on the `tenant_scoped` fixture. ## Ratchets `test_c27_strict_fleet_scoping` pins two things, and the probe is a fifth fleet-scoped read: * it must go through `_fleet_scope_clause`, never a hand-rolled `fleet_id.in_(fleet_ids)` — "exactly how A54 leaked". It does. * the helper's call-site count moves 5 -> 6, with the reasoning recorded at the constant. `strict=False` on purpose: non-strict is a SUPERSET (it also admits tenant-shared null-fleet rows and `scope_org`), so the probe stays at least as permissive as the search it explains and can never report "no memories" for rows the search could see. An earlier draft short-circuited ahead of `enforce_fleet_read_many` and the usage metering — a caller naming a fleet it has no rights to got a 200 with a warning instead of a 403, and the warning disclosed whether an agent id exists. `test_c27_strict_fleet_scoping` and `test_h05_multi_fleet_read_gate` caught it. That whole class is gone with the short-circuit: the search now always runs. ## Which field is explained (review round 2) Keyed on `filter_agent_id` whenever it is set, not on the resolved identity. `_resolve_read_identity` resolves `caller_agent_id or filter_agent_id`, but only `filter_agent_id` becomes `WHERE memories.agent_id = ?` — so keying on the identity explained the wrong id when a caller sent a valid `caller_agent_id` beside a typo'd filter, and the typo went unreported. Measured: filter=TYPO only -> ['filter_agent_unknown'] caller=REAL + filter=TYPO -> [] <-- the gap caller=REAL + filter=TYPO -> ['filter_agent_empty'] naming 'agnet-real' (fixed) Two knock-on corrections the first cut of this fix needed: * `agent_preexisted` describes the RESOLVED identity, which is not always the id being explained. Applying one agent's registration state to another reported a typo'd filter as "registered but empty", so `preexistence_of` now names which id the flag is about and the free signal is used only when it matches. Otherwise the probe is asked, which is one query on a path that already returned nothing. * `details` hardcoded `filter_agent_id`, which was wrong whenever the id came from `caller_agent_id` — a client acting on it would have corrected the wrong knob. It now carries `field` plus that field's own key. `caller_agent_id` alone still reports, but as visibility rather than filtering: it restricts no rows, it only decides which `scope_agent` rows are visible, so its message must not claim to have matched nothing. ## `/recall` too It parses the same `SearchRequest` and had the identical silent empty. Leaving it out is how the two routes came to disagree about what these fields mean — the reason `_resolve_read_identity` is shared at all. Its response gains `warnings`, seeded from the probe and then extended by the pipeline, so the field means what `/search`'s does rather than carrying only this one code. ## Known limitation, pre-existing The read path registers the asserted id via `get_or_create_agent`, so a repeated typo is registered by the first call and the second reports `filter_agent_empty` rather than `filter_agent_unknown`. Both still say the filter matched nothing, and the wording is advisory. Not introduced here and not fixed here: the clean answers are to stop registering on reads, or agent soft-delete (a `deleted_at` plus making `uq_agents_tenant_agent` partial, or re-registration breaks) — both larger than a warning warrants. ## Verification - `tests/test_caura723_agent_scope_warning.py` — 12 cases: the four states, the fleet-narrowed probe, `/recall`, the agent-scoped 403 left unchanged, no-filter and good-filter silence, a failing probe leaving search working, the no-cost-on-success contract, and both field-keying cases (a typo'd filter beside a valid caller id, and a caller id alone reported as visibility rather than filtering). - Targeted sweep post-rebase: 110 passed (`test_c27_strict_fleet_scoping`, `test_h05_multi_fleet_read_gate`, `test_route_authz_gaps`, `test_search_recall_tracked_flag`, `test_c4_recall_items_alias`, `test_mcp_recall`, `test_agent_admin_authz`). - Full `tests/`: 39 failures, byte-identical to `main`'s set on this machine (pre-existing FTS / relation-weight / request-observation env failures). `core-storage-api/tests/`: 369 passed, same 2 pre-existing CORS failures. - ruff check and format clean; mypy identical to `main` on every changed file. `do_not_touch_sentinel` all 35 survive, `tenant_scope_gate` exit 0, `legacy_name_ratchet` **"No new lines."** — the test reaches the dual-read Settings field through `tests/_legacy_contracts`' `LEGACY_API_KEY_FIELD` rather than minting the literal, in keeping with #1436-#1438. No schema changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Eyal Blyachman <eyal.b@caura.ai>

github-actions · 2026-09-09T16:37:21Z

Claude Code Review — skipped: PR author 'eyal-bl' is not a public member of the 'caura-ai' org

eyal-bl · 2026-09-09T16:42:40Z

@claude

github-actions · 2026-09-09T16:49:06Z

🤖 Review by Claude Code

Summary

This PR adds a diagnostic warning (filter_agent_unknown / filter_agent_empty / filter_agent_deregistered) when an agent-scoped search/recall comes back empty, using get_or_create_agent's pre-existence check as a free signal to avoid re-querying storage. The engineering around cost, ordering, and fleet-scope safety is careful and well-documented, but the core signal the feature relies on is undermined by a side effect of the very call it reads that signal from.

High Issues

`get_or_create_agent`'s auto-registration silently downgrades the "unknown agent" warning after the first occurrence of a typo

Severity: High
File: core-api/src/core_api/routes/memories.py:1899-1912 (and the mirrored block in recall_endpoint), core-api/src/core_api/services/agent_service.py:38-58, core-api/src/core_api/services/agent_scope.py:1-60
Problem: get_or_create_agent is called (and creates an agents row) for eff_agent_id before the search runs on every request, so after the very first search that uses a mistyped filter_agent_id/caller_agent_id, that typo becomes a permanently "registered" agent — meaning every subsequent identical request reports the softer, misleading filter_agent_empty ("registered but empty") instead of filter_agent_unknown ("check for a typo"), defeating the feature's own motivating scenario.

🤖 Claude Code Prompt

In core-api/src/core_api/routes/memories.py, both `_search_inner` (around lines
1899-1912) and `recall_endpoint` (around lines 2206-2219) call
`get_or_create_agent(body.tenant_id, eff_agent_id, fleet_id_hint,
registration_ctx=_agent_reg)` for the resolved identity *before* the search
runs. `get_or_create_agent` (core-api/src/core_api/services/agent_service.py,
lines 18-58) creates an `agents` row on first encounter of any agent_id,
including ones supplied only as a `filter_agent_id`/`caller_agent_id` on a
read-only search — there is no write associated with that id.

This means core_api/src/core_api/services/agent_scope.py's `explain_agent_scope`
(and the FILTER_AGENT_UNKNOWN vs FILTER_AGENT_EMPTY distinction it computes,
based on `agent_preexisted`/`known_preexisted`) only produces the correct
"unknown agent, check for a typo" diagnosis on the FIRST request that ever uses
a given wrong id in a tenant. Every subsequent request with the exact same
typo'd id sees `agent_preexisted=True` (since the row was auto-created by the
first request's own `get_or_create_agent` call) and gets downgraded to
FILTER_AGENT_EMPTY ("agent is registered but has no memories"), which reads as
a normal, benign state rather than a misconfiguration. This directly undercuts
the PR's own motivating case — a benchmark harness or integration that reuses
the same wrong agent id across many requests (the PR's docstring cites a
589-query run) — since only request #1 of that run would get a useful warning;
the remaining requests would get a warning that looks unremarkable.

Fix by not conflating "the agent row exists" with "the agent id is a genuine,
recognized identity for this caller's purposes" for ids introduced solely as
search/recall filters. Options to consider: (1) do not auto-register an agent
purely from a read-path `filter_agent_id`/`caller_agent_id` assertion — only
register on an actual write path (creation via `get_or_create_agent` should
arguably require a real memory write, not merely being named in a search); (2)
if lazy registration-on-read must stay, track something more meaningful than
row existence — e.g., whether the agent has ever written anything (which
`has_memories` in the probe already answers) — as the signal for "known agent"
rather than reusing `get_or_create_agent`'s preexistence flag; (3) at minimum,
add a regression test that issues the same wrong `filter_agent_id` twice
against a fresh tenant and asserts the SECOND request still reports
FILTER_AGENT_UNKNOWN, to make this failure mode visible if it currently is not
(it currently is not — none of the tests in
tests/test_caura723_agent_scope_warning.py issue a repeated search with the
same offending id).

Medium/Low Issues

`caller_agent_id`-only warning can misattribute an unrelated empty result

Severity: Low
File: core-api/src/core_api/services/agent_scope.py:180-232
Problem: When only caller_agent_id is set and the whole search returns nothing, FILTER_AGENT_EMPTY/FILTER_AGENT_UNKNOWN is emitted based solely on whether that agent has ever written anything — even though caller_agent_id restricts no rows and may be completely unrelated to why the broader tenant-wide search matched nothing (e.g., no query match at all), potentially steering a caller to "fix" the wrong field.

🤖 Claude Code Prompt

In core-api/src/core_api/services/agent_scope.py, `explain_agent_scope`
(lines ~180-232) attaches a `filter_agent_empty`/`filter_agent_unknown`
warning whenever `caller_agent_id` alone is set, the overall search/recall
returned zero results, and the named agent has no memories of its own —
regardless of whether the tenant-wide search would have returned zero results
anyway for reasons unrelated to `caller_agent_id` (e.g., the query simply
doesn't match anything in the tenant). Since `caller_agent_id` only widens
visibility of that agent's own scope_agent rows and does not filter the main
result set, the resulting warning can misleadingly correlate an unrelated
empty result with the caller_agent_id, even though the wording was
deliberately softened ("grants visibility of no scope_agent rows" rather than
"matched nothing"). Consider gating this warning on something closer to
whether the empty result plausibly depended on scope_agent visibility (e.g.,
only surface it when the tenant otherwise has content the caller could
plausibly want, or drop it when `filter_agent_id`/other filters are entirely
absent and the corpus itself is empty) or clarify in the message that this is
an independent observation, not necessarily the cause of the empty result.

Reviewed by claude-sonnet-5 · cost $1.8045084999999998

eyal-bl · 2026-09-09T17:04:41Z

@claude

github-actions · 2026-09-09T17:11:18Z

🤖 Review by Claude Code

Summary

This PR adds a well-documented "agent scope" diagnostic (CAURA-723) that explains empty search/recall results caused by an unknown or deregistered filter_agent_id/caller_agent_id. The implementation is careful about cost (probe only fires on empty results), about the deregistration edge case (memories outlive DELETE /agents/{id}), and about which field to key the explanation on. I found one real correctness gap around cross-tenant reads that can produce a misleading warning; everything else checked out (fail-open probe, fleet-scope superset direction, response-model field compatibility, backward-compatible storage endpoint default).

Medium/Low Issues

Cross-tenant reads: peer-tenant agents get mislabeled as "deregistered"

Severity: Medium
File: core-api/src/core_api/services/agent_scope.py:_deregistered, core-storage-api/src/core_storage_api/services/postgres_service.py (memory_agent_scope_probe, agent_registered lookup)
Problem: agent_registered/preexisted is always checked against the home tenant_id only, so for a cross-tenant read where the filtered agent genuinely belongs to a different (but readable) tenant, has_memories=True + registered=False triggers FILTER_AGENT_DEREGISTERED — wrongly claiming the agent "was deregistered, or predates agent tracking" when it is actually a normally-registered agent that simply isn't in the queried home tenant's agents table.

🤖 Claude Code Prompt

In core-api/src/core_api/services/agent_scope.py, the `_deregistered()` helper
(used both in the `had_results=True` branch and the `has_memories=True` branch
of `explain_agent_scope()`) always attributes a "not registered in this tenant"
signal to deregistration or pre-tracking legacy data. That reasoning ignores a
third, legitimate cause: cross-tenant reads.

In core-storage-api/src/core_storage_api/services/postgres_service.py,
`memory_agent_scope_probe()` computes `has_memories` over the SUPERSET scope
(`readable_tenant_ids` when present, i.e. potentially several tenants), but
computes `agent_registered` scoped ONLY to the single `tenant_id` param (the
"home tenant" the query names) via
`select(Agent.id).where(Agent.tenant_id == tenant_id, Agent.agent_id == agent_id)`.
When a cross-tenant caller (auth.is_cross_tenant_read) filters by an agent id
that is genuinely registered in a *different* readable tenant and has memories
there, `has_memories` comes back True (because the superset query reaches that
tenant's rows) while `agent_registered` comes back False (because the lookup
never checks the peer tenant). `explain_agent_scope()` in agent_scope.py then
returns `_deregistered(...)`, whose message says the agent "has no registration
in this tenant... It was deregistered, or predates agent tracking" — which is
wrong and misleading for this case; the agent is registered, just under a peer
tenant the reader is allowed to see.

This is a real gap: the `FILTER_AGENT_UNKNOWN` branch already accounts for the
"belongs to a different tenant" possibility in its message text ("Check the id
for a typo, or whether it belongs to a different tenant"), but the
`FILTER_AGENT_DEREGISTERED` branch does not, even though it is reachable via
the exact same cross-tenant read path (readable_tenant_ids is threaded into
both `explain_agent_scope()` and the probe payload specifically to support
cross-tenant search).

Fix by either: (a) having `memory_agent_scope_probe` also check
`agent_registered` across `readable_tenant_ids` (or return which tenant it
belongs to) so the caller can distinguish "genuinely gone" from "registered
elsewhere", or (b) softening the `_deregistered()` message to include the
"or belongs to a different tenant in your read scope" possibility whenever
`readable_tenant_ids` was supplied to `explain_agent_scope`, similar to how
the unknown-agent message already hedges. Add a regression test exercising a
cross-tenant read where the filtered agent is registered and has memories only
in a peer (non-home) tenant, asserting the warning is not `filter_agent_deregistered`
with the current misleading wording.

Reviewed by claude-sonnet-5 · cost $2.3622207499999996

github-actions · 2026-09-10T10:27:31Z

Claude Code Review — skipped: PR author 'eyal-bl' is not a public member of the 'caura-ai' org

…lt is empty A tenant-scoped caller that passes a wrong `filter_agent_id` gets `HTTP 200 · items [] · warnings null` — byte-identical to a correct id that simply has nothing relevant to say. Measured, one memory written by `agent-real`: filter_agent_id = "agent-real" -> 200 items=1 filter_agent_id = "agnet-real" -> 200 items=0 warnings=None The filter is a SQL `WHERE memories.agent_id = ?`, so a typo matches no rows and the query returns nothing exactly as an empty query would. It fails in the safe-looking direction — an empty list reads as "no data yet", the most ordinary thing a memory product can say, so nobody investigates. It cost a 589-query benchmark run against an empty store. ## Scope: a tenant-scoped-caller bug only An agent-scoped credential carries a verified `X-Agent-ID`, and `enforce_self_agent` **403s** it for naming any agent but itself — measured, for a typo and for a peer id. So the silent path needs a credential with no agent identity, asserting the id in the body. Not a corner: it is the dashboard's hand-typed `filter_agent_id` text box (tenant-scoped by its own comment, "no X-Agent-ID"), the documented `caura_recall` parameter on the public for-agents page, every benchmark harness we own, and the multi-user backend shape where one key serves many end users. ## When it runs, and what it costs Only after the search, and the storage probe only when the search came back EMPTY. **A successful agent-filtered search pays nothing** — no extra round trip, no extra query. Pinned by `test_a_successful_search_makes_no_probe_call`, which counts calls rather than trusting the reading. This is the second attempt, after review of #1434. The first probed BEFORE the search to skip the embedding call on a misconfigured request — but that saving lands on requests already broken, which are rare, while the cost lands on every healthy one, which is the norm. Wrong way round on expected cost. The review also caught that the docstring claimed the probe "runs only when a search is about to return nothing", which was false of that ordering; it is true of this one. The empty path is now one query, not two: `include_agent_registered` lets the probe skip the `agents` lookup, because the route already made it (see below). ## Which fact decides what, and why not the obvious one The natural implementation — look the id up in `agents`, and if absent report "no such agent" — is wrong in the direction that misleads. `DELETE /agents/{id}` says so itself: *"Delete an agent. Memories written by this agent are NOT deleted."* The rows stay live and searchable with no agent row, and rows predating agent tracking were never registered at all. A missing agent row means "deregistered or never registered", never "nothing to find". So `has_memories` establishes there was nothing to find, and `agent_preexisted` only chooses the wording: returned rows + agent gone -> filter_agent_deregistered (free) empty + has_memories + gone -> filter_agent_deregistered empty + no memories + known -> filter_agent_empty empty + no memories + unknown -> filter_agent_unknown anything else -> silence `agent_preexisted` comes from the route's own `get_or_create_agent`, via a new `registration_ctx` out-dict in the same shape as `diagnostic_ctx` / `warnings_ctx` / `recall_ctx`. That call already does the `agents` lookup, so it is free — and it *has* to come from there, because by the time the search has run the row exists whether or not it did beforehand, and asking afterwards would report every typo as a registered agent. An out-dict rather than a changed return type so the other seven callers stay untouched. Known boundary: an admin credential skips `get_or_create_agent` (`if auth.tenant_id:`), so it has no free pre-existence signal and is not told an agent is deregistered on a NON-empty result. Buying that would put a query back on the success path. Documented on the `tenant_scoped` fixture. ## Ratchets `test_c27_strict_fleet_scoping` pins two things, and the probe is a fifth fleet-scoped read: * it must go through `_fleet_scope_clause`, never a hand-rolled `fleet_id.in_(fleet_ids)` — "exactly how A54 leaked". It does. * the helper's call-site count moves 5 -> 6, with the reasoning recorded at the constant. `strict=False` on purpose: non-strict is a SUPERSET (it also admits tenant-shared null-fleet rows and `scope_org`), so the probe stays at least as permissive as the search it explains and can never report "no memories" for rows the search could see. An earlier draft short-circuited ahead of `enforce_fleet_read_many` and the usage metering — a caller naming a fleet it has no rights to got a 200 with a warning instead of a 403, and the warning disclosed whether an agent id exists. `test_c27_strict_fleet_scoping` and `test_h05_multi_fleet_read_gate` caught it. That whole class is gone with the short-circuit: the search now always runs. ## Which field is explained (review round 2) Keyed on `filter_agent_id` whenever it is set, not on the resolved identity. `_resolve_read_identity` resolves `caller_agent_id or filter_agent_id`, but only `filter_agent_id` becomes `WHERE memories.agent_id = ?` — so keying on the identity explained the wrong id when a caller sent a valid `caller_agent_id` beside a typo'd filter, and the typo went unreported. Measured: filter=TYPO only -> ['filter_agent_unknown'] caller=REAL + filter=TYPO -> [] <-- the gap caller=REAL + filter=TYPO -> ['filter_agent_empty'] naming 'agnet-real' (fixed) Two knock-on corrections the first cut of this fix needed: * `agent_preexisted` describes the RESOLVED identity, which is not always the id being explained. Applying one agent's registration state to another reported a typo'd filter as "registered but empty", so `preexistence_of` now names which id the flag is about and the free signal is used only when it matches. Otherwise the probe is asked, which is one query on a path that already returned nothing. * `details` hardcoded `filter_agent_id`, which was wrong whenever the id came from `caller_agent_id` — a client acting on it would have corrected the wrong knob. It now carries `field` plus that field's own key. `caller_agent_id` alone still reports, but as visibility rather than filtering: it restricts no rows, it only decides which `scope_agent` rows are visible, so its message must not claim to have matched nothing. ## `/recall` too It parses the same `SearchRequest` and had the identical silent empty. Leaving it out is how the two routes came to disagree about what these fields mean — the reason `_resolve_read_identity` is shared at all. Its response gains `warnings`, seeded from the probe and then extended by the pipeline, so the field means what `/search`'s does rather than carrying only this one code. ## Known limitation, pre-existing — and why the wording carries it `get_or_create_agent` runs on the READ path and creates a row for whatever id was asserted, so a search mints agent rows from free-text input. The first request carrying a typo reports `filter_agent_unknown` correctly and, in doing so, creates the row that makes the next one report `filter_agent_empty`: 1st search, filter_agent_id="agnet-real" -> filter_agent_unknown 2nd search, same typo -> filter_agent_empty A harness reusing one wrong id gets the sharp signal once and the soft one thereafter — the exact shape of the incident behind this work. An earlier revision noted this and called the wording "advisory", which under-rated it: "agent is registered but has no memories" reads as a benign new-agent state, so a caller takes "registered" for "the id is right" and stops looking. That is the outcome the warning exists to prevent, so the message now says outright that registration is weak evidence and that a repeated typo lands there. Both codes are kept because the first-occurrence signal is accurate and worth having. Pinned by `test_a_repeated_typo_still_warns_and_never_reads_as_benign`, which asserts the second request still warns, still names the offending id, and does not read as benign — a test rather than a commit note, so a future change to registration makes it visible either way. Not fixable from here. The root fix is to stop registering on reads, and it is security-adjacent: the route needs the row for trust-level fleet forcing and `enforce_fleet_read_many`, so "don't create it" first requires deciding what trust and fleet apply to an unknown agent. Tracked as CAURA-724, which also covers the two consequences beyond this warning — a read with a write side effect, and a tenant key being able to mint unlimited agent rows one search at a time. ## Verification - `tests/test_caura723_agent_scope_warning.py` — 13 cases: the four states, the fleet-narrowed probe, `/recall`, the agent-scoped 403 left unchanged, no-filter and good-filter silence, a failing probe leaving search working, the no-cost-on-success contract, and both field-keying cases (a typo'd filter beside a valid caller id, and a caller id alone reported as visibility rather than filtering), and the repeated-typo downgrade. - Targeted sweep post-rebase: 110 passed (`test_c27_strict_fleet_scoping`, `test_h05_multi_fleet_read_gate`, `test_route_authz_gaps`, `test_search_recall_tracked_flag`, `test_c4_recall_items_alias`, `test_mcp_recall`, `test_agent_admin_authz`). - Full `tests/`: 39 failures, byte-identical to `main`'s set on this machine (pre-existing FTS / relation-weight / request-observation env failures). `core-storage-api/tests/`: 369 passed, same 2 pre-existing CORS failures. - ruff check and format clean; mypy identical to `main` on every changed file. `do_not_touch_sentinel` all 35 survive, `tenant_scope_gate` exit 0, `legacy_name_ratchet` **"No new lines."** — the test reaches the dual-read Settings field through `tests/_legacy_contracts`' `LEGACY_API_KEY_FIELD` rather than minting the literal, in keeping with #1436-#1438. No schema changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Eyal Blyachman <eyal.b@caura.ai>

github-actions · 2026-09-10T10:40:17Z

Claude Code Review — skipped: PR author 'eyal-bl' is not a public member of the 'caura-ai' org

eyal-bl · 2026-09-10T10:41:41Z

@claude

github-actions · 2026-09-10T10:48:04Z

🤖 Review by Claude Code

Summary

This PR adds a well-documented, thoughtfully-tested feature (CAURA-723) that explains why an agent-filtered search/recall returned an empty result. The core logic (probe-only-on-empty, filter_agent_id vs caller_agent_id precedence, agents-row-deletion-doesn't-delete-memories handling) is sound and matches its extensive design docs. I found two minor real issues, both low-impact.

Medium/Low Issues

`/recall`'s `warnings` field is omitted rather than null on the empty path, unlike `/search`

Severity: Low
File: core-api/src/core_api/routes/memories.py:2342-2346 (and core-api/src/core_api/openapi_responses.py:120-129)
Problem: /search always serializes warnings (explicit null when empty, via the Pydantic SearchResponse model), but /recall only sets the "warnings" key on the returned dict when recall_warnings is non-empty (if recall_warnings: recall_response["warnings"] = recall_warnings), so the key is entirely absent on the common empty-result case — contradicting both the inline comment ("so a caller that reads the field sees the same shape on both routes") and the RecallResponse.warnings docstring ("Null when there is nothing to say").

🤖 Claude Code Prompt

In core-api/src/core_api/routes/memories.py, around line 2342-2346 in recall_endpoint,
change:

    if recall_warnings:
        recall_response["warnings"] = recall_warnings
    return recall_response

to always set the key, e.g.:

    recall_response["warnings"] = recall_warnings or None
    return recall_response

so the /recall response always carries a "warnings" key (null when empty), matching
/search's SearchResponse.warnings behavior and the documentation in
core-api/src/core_api/openapi_responses.py's RecallResponse.warnings field
("Null when there is nothing to say").

Agent-scope-probe latency is not captured in the search-completion timing log

Severity: Low
File: core-api/src/core_api/routes/memories.py:1978-1990, 1997-2011
Problem: The finally: block that logs total_ms for the search request runs before the new explain_agent_scope(...) call; on exactly the requests this feature targets (empty results, e.g. a typo'd filter_agent_id), an extra HTTP round trip to storage now happens after that log line, so total_ms under-reports actual request latency for the affected bucket — the case most likely to be scrutinized by latency monitoring/alerting.

🤖 Claude Code Prompt

In core-api/src/core_api/routes/memories.py, `_search_inner` logs "search request
completed" with total_ms inside the `finally:` block ending around line 1990, but the
new CAURA-723 `explain_agent_scope(...)` call (added after that block, around
line 1997-2011) runs afterward and can add a real HTTP round trip to storage when
results are empty. Either move the timing log to after the explain_agent_scope call
(recomputing total_ms at that point), or record a second timestamp/duration for the
probe and include it in the log's extra fields, so total_ms reflects true end-to-end
latency for empty-result search requests instead of undercounting them.

Reviewed by claude-sonnet-5 · cost $1.8993170000000001

eyal-bl requested a review from a team as a code owner September 9, 2026 12:59

eyal-bl force-pushed the CAURA-723-signal-bad-agent-id branch from 223d246 to 072f6b4 Compare September 9, 2026 14:15

eyal-bl force-pushed the CAURA-723-signal-bad-agent-id branch from f8e0abf to e7e0798 Compare September 9, 2026 16:37

eyal-bl force-pushed the CAURA-723-signal-bad-agent-id branch from 6c04592 to dbf54c0 Compare September 10, 2026 10:40

Conversation

eyal-bl commented Sep 9, 2026

Scope: this is a tenant-scoped-caller bug only

Which table decides what, and why not the obvious one

Ordering, and a hole the ordering first created

Cost

Ratchets moved, both deliberately

/recall too

Known limitation, pre-existing

Verification

Summary

Related Issue

Type of Change

How Has This Been Tested?

Checklist

Additional Notes

Uh oh!

github-actions Bot commented Sep 9, 2026

Uh oh!

eyal-bl commented Sep 9, 2026

Uh oh!

github-actions Bot commented Sep 9, 2026

Uh oh!

github-actions Bot commented Sep 9, 2026

Summary

Medium/Low Issues

Probe adds an unconditional extra round-trip to every filtered search/recall, contradicting its own docstring

Uh oh!

github-actions Bot commented Sep 9, 2026

Uh oh!

github-actions Bot commented Sep 9, 2026

Uh oh!

github-actions Bot commented Sep 9, 2026

Uh oh!

eyal-bl commented Sep 9, 2026

Uh oh!

github-actions Bot commented Sep 9, 2026

Summary

Medium/Low Issues

Agent-scope warning is keyed on the resolved identity, not the actual filter_agent_id, so it can misidentify or miss the real cause of an empty result

Uh oh!

github-actions Bot commented Sep 9, 2026

Uh oh!

eyal-bl commented Sep 9, 2026

Uh oh!

github-actions Bot commented Sep 9, 2026

Summary

High Issues

get_or_create_agent's auto-registration silently downgrades the "unknown agent" warning after the first occurrence of a typo

Medium/Low Issues

caller_agent_id-only warning can misattribute an unrelated empty result

Uh oh!

eyal-bl commented Sep 9, 2026

Uh oh!

github-actions Bot commented Sep 9, 2026

Summary

Medium/Low Issues

Cross-tenant reads: peer-tenant agents get mislabeled as "deregistered"

Uh oh!

github-actions Bot commented Sep 10, 2026

Uh oh!

github-actions Bot commented Sep 10, 2026

Uh oh!

eyal-bl commented Sep 10, 2026

Uh oh!

github-actions Bot commented Sep 10, 2026

Summary

Medium/Low Issues

/recall's warnings field is omitted rather than null on the empty path, unlike /search

Agent-scope-probe latency is not captured in the search-completion timing log

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

`/recall` too

Agent-scope warning is keyed on the resolved identity, not the actual `filter_agent_id`, so it can misidentify or miss the real cause of an empty result

`get_or_create_agent`'s auto-registration silently downgrades the "unknown agent" warning after the first occurrence of a typo

`caller_agent_id`-only warning can misattribute an unrelated empty result

`/recall`'s `warnings` field is omitted rather than null on the empty path, unlike `/search`