fix(dev-api): serve legacy memories for a legacy-cohort vector search (#10203)

[aryanorastar]

What

Closes #10203.

GET /v1/dev/user/memories/vector/search failed closed with 403 missing_rollout_state for un-enrolled legacy-cohort accounts — even though the sibling read paths already recover for exactly these accounts:

• the memory list GET /v1/dev/user/memories (Developer API: GET /v1/dev/user/memories returns 403 missing_rollout_state (conversations/action-items OK) #9892 / fix(dev): serve legacy memories when a legacy-cohort account has no rollout state #10094), and
• the MCP memory path (fix(mcp): serve legacy memories when a legacy-cohort account has no rollout state #10095),

both serve the legacy memories collection when pin_memory_system has resolved the account to LEGACY and the only signal is an absent rollout doc. So an affected account could list its memories but not vector-search them — the "another rollout-state path still failing closed" cohort the issue asks about (possibility #3).

Worse, search_memories_vector raised 403 on should_use_legacy_fallback (USE_LEGACY_SAFE) — the one signal that explicitly means serve legacy.

Note on the user's report: the list endpoint (GET /v1/dev/user/memories) is already fixed in code, so a user still seeing 403 there is a deploy-lag symptom (issue possibility #1), not a code path. This PR closes the genuine remaining code gap — the vector endpoint.

Fix

On a legacy-cohort signal — USE_LEGACY_SAFE, or a deny whose only reason is missing_rollout_state — search the legacy memories vector index (search_memories_by_vector) and hydrate the hits (get_memories_by_ids), instead of returning 403:

• vector-relevance order preserved (iterate the ranked IDs, not hydration order);
• locked content redacted exactly as the list route does;
• relevance_score left null (the legacy index doesn't expose scores);
• record_fallback(... to_mode='legacy_memories', outcome='recovered') marks the recovery;
• any other deny reason keeps the fail-closed 403.

Why not a shared predicate (yet)

This is the 4th instance of one cause — a default-read surface failing closed for an un-enrolled legacy account instead of serving legacy (#9892 → #10094 list, #10095 MCP, this = vector). The ideal class-level guard is one predicate every read surface calls. But get_memories reaches legacy by fallthrough while the vector route returns explicitly, so unifying them ripples into the list route's control flow and its static tripwire. Per AGENTS.md ("land the enforceable guard now, track high-blast-radius follow-up explicitly") I land the focused fix plus the updated tripwire (which now asserts the vector route recovers on the shared signal) and flag the shared-predicate extraction as a follow-up.

Verification

Ran with a real backend venv (Python 3.11):

test_dev_api_canonical_grant_ordering.py ......... 9 passed
  incl. 3 new vector cases:
    - missing_rollout_state → legacy fallback (200), vector order preserved
    - USE_LEGACY_SAFE       → legacy fallback (200)
    - other deny reason     → still 403
mutation: revert the fix → the 2 fallback cases fail, the 403 case still passes
144 passed across every vector-referencing unit test

The updated test_developer_routes_only_reach_legacy_after_explicit_legacy_safe_decision tripwire now encodes the new (consistent) contract for both routes.

Failure-Class: none

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[Git-on-my-level]

Thanks for the focused fix here — the fallback is narrow, keeps other deny reasons fail-closed, preserves legacy vector ordering, and the added unit coverage targets the right regression cases.

I’m requesting changes for one concrete merge blocker: the Public Developer API contract check is failing because docs/api-reference/app-client-openapi.json is stale after this Developer API route change. Please regenerate and commit the app-client OpenAPI contract (backend/scripts/export_openapi.py --write ../docs/api-reference/app-client-openapi.json, or the repo’s equivalent wrapper) so CI is green.

Given this touches Developer API memory reads/vector search over user memories and legacy rollout fallback behavior, I’d still want a human maintainer to sanity-check the rollout/data-handling semantics before merge, but I don’t see a security/supply-chain issue in the diff itself.

---

by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.

[kodjima33]

backend dev-api bug fix (serve legacy-cohort memories, #10203) — approve only (CHANGES_REQUESTED)

[aryanorastar]

Thanks — but this diff doesn't change the app-client contract. I regenerated it on this branch to check: the result is +104 lines that are entirely the memories baseline route/field from #8728 (PATCH …/baseline, is_baseline), and zero from this PR's vector/search change — the fix is control-flow only (serve legacy vs 403), the DeveloperMemoryVectorSearchResponse model is untouched.

So app-client-openapi.json is stale on main from #8728, not from here — the OpenAPI Contract job is red on every backend PR, not just this one.

The catch with regenerating it in this PR: that single job also runs the Dart/TS/Swift client --check steps, so going green needs the whole regenerated chain — including Desktop/Sources/Generated/OmiApi.generated.swift, which then trips the desktop-changelog gate. That's exactly why I split it into #10482 (the full spec + client-type regen; #10481 exempts generated Swift from the changelog gate so it can carry the Swift file).

Once #10482 lands, this goes green with no change. Since @kodjima33 has already approved the substance, could we either merge #10482 first, or treat this OpenAPI-contract failure as the pre-existing #8728 drift it is? Happy to rebase after #10482 to confirm green.

OpenAPI/Public Developer API contract blocker from this automation review is resolved on current head 67fbc87; checks now pass. Human maintainer review label remains.

[Git-on-my-level]

Thanks for the follow-up here. I re-checked the current head and the previous generated-contract blocker is no longer blocking: the Public Developer API contract and backend checks are passing now, so I dismissed my stale automation change-request review.

Implementation-wise this still looks like a narrow bug fix: auth/default-memory grant checks remain ahead of the memory-system branch, legacy serving is limited to USE_LEGACY_SAFE or the missing_rollout_state legacy-cohort case, other deny reasons still fail closed, and the tests cover vector-order preservation plus both fallback and deny paths.

Because this touches Developer API vector search over user memories and legacy rollout fallback semantics, I’m leaving this as a positive signal rather than a formal approval; the existing human maintainer review label is still appropriate before merge.

---

by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.

[kodjima33]

Legacy-cohort accounts could list but not vector-search memories (#10203); mirrors the existing listing/MCP fallback. Tested.