Skip to content

fix(dev): serve legacy memories when a legacy-cohort account has no rollout state#10094

Merged
undivisible merged 1 commit into
BasedHardware:mainfrom
aryanorastar:fix/9892-developer-api-memories-rollout
Jul 20, 2026
Merged

fix(dev): serve legacy memories when a legacy-cohort account has no rollout state#10094
undivisible merged 1 commit into
BasedHardware:mainfrom
aryanorastar:fix/9892-developer-api-memories-rollout

Conversation

@aryanorastar

@aryanorastar aryanorastar commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Summary

  • GET /v1/dev/user/memories no longer returns 403 missing_rollout_state for accounts that were never enrolled in the canonical-memory rollout: the route now serves the authoritative legacy memories collection for them.
  • Every other deny reason (missing grant, malformed state, unsupported schema, uid mismatch) keeps the fail-closed 403 contract, as does /v1/dev/user/memories/vector/search, which has no legacy read surface.
  • The mode change is recorded through the shared record_fallback helper (component=other, reason=policy, outcome=recovered) — no new one-off counter.

Fixes #9892.

Root cause

Two authorities disagreed about the same account, and the fail-closed one won:

  • Cohort selector (authoritative): pin_memory_systemresolve_memory_system resolves everyone outside the code-reviewed CANONICAL_MEMORY_USERS whitelist to MemorySystem.LEGACY — "the documented default — not an implicit None fallback" (its own words). For these accounts, users/{uid}/memory_control/state is expected to be absent.
  • Rollout gate: read_default_read_rollout treats an absent doc as DENY_MEMORY / missing_rollout_state — correct fail-closed behavior for an account that might be mid-migration, wrong for one the cohort selector already pinned LEGACY in the same request.

The route's legacy read path (memories_db.get_memories + per-doc validation) already existed but was unreachable: only USE_LEGACY_SAFE reaches it, and rollout normalization never produces USE_LEGACY_SAFE from Firestore state — only the explicit opt-in constructor does. So every un-enrolled account lost Developer API memory reads entirely, while /v1/dev/user/conversations and /v1/dev/user/action-items (no such gate) kept working — exactly the reported symptom, breaking e.g. the Obsidian omi-sync memories feed.

Failure class

Failure-Class: none

No registered class covers a reader failing closed on state whose absence the authoritative cohort selector defines as normal. It rhymes with FC-split-mutation-authority (competing authorities over one decision) but that class is scoped to mutable-state transition ownership, not read-side policy divergence; flagging rather than stretching — same call as #10078.

Durable guard

  • The fallback is narrow and explicit: only missing_rollout_state (doc absent = never enrolled) on the already-LEGACY branch falls back. malformed_rollout_state, rollout_read_failed, grant and schema denials all stay fail-closed, because there a doc exists (or its state is unknown) and the account may be mid-migration.
  • The previously silent-dead should_use_legacy_fallback: pass branch is gone; the decision structure now states every outcome.
  • Mode change is observable via the shared omi_fallback_total metric instead of being invisible.

Product invariants affected

none (verified with scripts/pr-preflight --suggest)

Validation

  • Old-code regression check (route change stashed): test_get_memories_missing_rollout_state_falls_back_to_legacyfails (403); with the fix → passes. Full file: 7 passed.
  • New tests extend the existing grant-ordering suite through its established seams: an un-enrolled legacy-cohort account lists legacy memories with 200 (grant check still runs first); a non-missing deny reason still 403s with the actionable developer_memory_access_not_ready contract; the vector-search missing-rollout 403 test keeps its contract untouched.
  • Repo runner (file-isolated, as CI executes): tests/unit/test_dev_api_canonical_grant_ordering.py → 7 passed, tests/unit/test_developer_memory_adapter.py → 24 passed, tests/unit/test_memories_validation.py → 12 passed, tests/unit/test_dev_api_folder_filters.py → included in sweep, passed.
  • The pre-push bounded gate ran the wider backend selection and passed; it caught (and I updated) the static source-order tripwire in test_developer_memory_adapter.py that encoded the old "no legacy after deny" ordering — it now asserts the narrow un-enrolled guard precedes the legacy read, and is labeled as a static tripwire per the repo's testing guidance.
  • make preflight (local lane, this PR body) → all selected checks pass.
  • black --line-length 120 --skip-string-normalization clean on both changed files.

Not exercised live: a production Developer API key against a real un-enrolled account (no such credentials here). The fix point is server-side branch selection driven directly by the regression tests; the issue reporter's account can verify live after deploy.

Out of scope, named

  • MCP list/search (routers/mcp.py, mcp_sse.py) degrade differently for the same accounts (silent empty list instead of 403). Same underlying divergence, different surface and files — follow-up rather than scope creep here.
  • The issue's alternative ask (enrolling individual accounts) is an ops action, not a code fix; this makes it unnecessary for reads.

Review in cubic

@chatgpt-codex-connector

Copy link
Copy Markdown

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

…ollout state

GET /v1/dev/user/memories fails closed with 403 missing_rollout_state for
any account whose users/{uid}/memory_control/state doc is absent — which
is the expected state for every account outside the canonical-memory
cohort whitelist. The route already resolved the authoritative cohort via
pin_memory_system (LEGACY) and already carries a legacy read path, but
that path was unreachable: rollout normalization never yields
USE_LEGACY_SAFE from Firestore state, so un-enrolled accounts lost
Developer API memory reads entirely (conversations and action items,
which have no such gate, kept working) (BasedHardware#9892).

The reader now treats missing_rollout_state on the legacy branch as the
un-enrolled case and serves the authoritative legacy memories collection,
recording the mode change through the shared record_fallback helper.
Every other deny reason (grant missing, malformed state, schema, uid
mismatch) keeps the fail-closed 403 contract, as does the vector search
endpoint, which has no legacy read surface.

Regression tests: an un-enrolled legacy-cohort account lists legacy
memories with 200; a non-missing deny reason still 403s with the
actionable error contract.

Fixes BasedHardware#9892.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Failure-Class: none
@undivisible
undivisible force-pushed the fix/9892-developer-api-memories-rollout branch from 3fc8bb7 to 9c59a04 Compare July 20, 2026 07:07
undivisible pushed a commit to aryanorastar/omi that referenced this pull request Jul 20, 2026
…ollout state

The MCP REST routes and SSE tools for memory list and search return an
empty result set for any account whose users/{uid}/memory_control/state
doc is absent — the expected state for every account outside the
canonical-memory cohort whitelist. All four sites reach that branch only
after pin_memory_system resolved the account to LEGACY, where the legacy
memories collection is the authoritative read surface, so the silent
empty result hid real data from un-enrolled accounts — the MCP-surface
form of the Developer API bug (BasedHardware#9892, fixed for that route in BasedHardware#10094).

The four sites now route their legacy-fallback decision through one
shared helper, mcp_legacy_read_authorized: an explicit legacy-safe
decision or the never-enrolled missing_rollout_state case serves the
legacy surface (recorded via the shared record_fallback helper); every
other deny reason (malformed state, read failure, missing grant,
shadow-only) stays fail-closed with an empty result.

Regression tests: behavioral coverage of the helper's decision table,
and the four source-order tripwires updated to assert the routes gate
legacy reads through the helper.

Fixes BasedHardware#9892 for the MCP surface.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@undivisible undivisible left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified rebased onto main, ratchet conflict resolved, local unit gates green, CI green.

Narrow unenrolled legacy-cohort fallback on GET /v1/dev/user/memories is correct: only missing_rollout_state after pin_memory_system LEGACY falls through; other denies stay 403. Vector route untouched. Failure-Class: none accepted.

Merge readiness: regular merge (no squash).

@undivisible
undivisible merged commit a2f50de into BasedHardware:main Jul 20, 2026
28 checks passed
undivisible pushed a commit to aryanorastar/omi that referenced this pull request Jul 20, 2026
…ollout state

The MCP REST routes and SSE tools for memory list and search return an
empty result set for any account whose users/{uid}/memory_control/state
doc is absent — the expected state for every account outside the
canonical-memory cohort whitelist. All four sites reach that branch only
after pin_memory_system resolved the account to LEGACY, where the legacy
memories collection is the authoritative read surface, so the silent
empty result hid real data from un-enrolled accounts — the MCP-surface
form of the Developer API bug (BasedHardware#9892, fixed for that route in BasedHardware#10094).

The four sites now route their legacy-fallback decision through one
shared helper, mcp_legacy_read_authorized: an explicit legacy-safe
decision or the never-enrolled missing_rollout_state case serves the
legacy surface (recorded via the shared record_fallback helper); every
other deny reason (malformed state, read failure, missing grant,
shadow-only) stays fail-closed with an empty result.

Regression tests: behavioral coverage of the helper's decision table,
and the four source-order tripwires updated to assert the routes gate
legacy reads through the helper.

Fixes BasedHardware#9892 for the MCP surface.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
undivisible added a commit that referenced this pull request Jul 20, 2026
…ollout state (#10095)

# Summary

- MCP memory reads (`GET /v1/mcp/memories`, `GET
/v1/mcp/memories/search`, and the SSE `get_memories` / `search_memories`
tools) no longer return a silent empty result for accounts that were
never enrolled in the canonical-memory rollout: they now serve the
authoritative legacy `memories` surface.
- All four sites route the decision through one shared helper,
`mcp_legacy_read_authorized`, instead of four copies of the `!=
USE_LEGACY_SAFE` comparison. Every other deny reason (malformed state,
read failure, missing grant, shadow-only) stays fail-closed.
- The mode change is recorded through the shared `record_fallback`
helper — no new one-off counter.

MCP-surface counterpart of #10094 (Developer API), same root cause.
Fixes #9892 for the MCP surface.

# Root cause

Same divergence as #10094: `pin_memory_system` resolves every account
outside the code-reviewed canonical whitelist to `MemorySystem.LEGACY`,
where `users/{uid}/memory_control/state` is *expected* to be absent —
but `read_default_read_rollout` treats the absent doc as `DENY_MEMORY` /
`missing_rollout_state`. The MCP sites reach that branch only after the
cohort pin already said LEGACY, then returned `[]` instead of reading
the legacy collection the selector had just declared authoritative. The
Developer API surfaced this as a 403; MCP hid it as an empty memories
list.

# Failure class

Failure-Class: none

Same call as #10094: no registered class covers a reader failing closed
on state whose absence the authoritative cohort selector defines as
normal, and FC-split-mutation-authority is scoped to mutable-state
transition ownership, not read-side policy.

# Durable guard

- One shared decision owner: the four call sites can no longer drift
apart — the narrow un-enrolled exception and its telemetry live in
`mcp_legacy_read_authorized`, whose docstring names the contract.
- The fallback is narrow: only `missing_rollout_state` (doc absent =
never enrolled) qualifies; `malformed_rollout_state` and
`rollout_read_failed` stay fail-closed because there the doc exists or
its state is unknown.
- The existing source-order tripwires (labeled as such) now assert all
four routes gate legacy reads through the helper.

# Product invariants affected

- INV-MEM-1 (path-matched via `utils/mcp_memories.py`): no tier
vocabulary change — the helper only decides legacy-vs-deny routing; tier
semantics and the guard test are untouched.
- INV-MEM-3 note: the new fallback runs only on the non-canonical branch
(cohort pin already resolved LEGACY); canonical selection still never
falls back to legacy.

# Validation

- Behavioral regression test of the helper's full decision table
(`test_mcp_legacy_read_authorized_only_for_explicit_or_unenrolled`):
explicit legacy-safe and never-enrolled pass;
malformed/read-failed/no-grant/shadow-only fail closed — this test
cannot pass on pre-fix code (the helper did not exist, and the old
inline comparisons denied the un-enrolled case).
- Repo runner (file-isolated, as CI executes):
`tests/unit/test_mcp_memory_adapter.py` +
`tests/unit/test_mcp_data_endpoints.py` → 27 passed.
- `make preflight` (local lane, this PR body) → all selected checks
pass.
- `black` clean on all changed files.
- `backend/routers/mcp_sse.py` frozen-size baseline raised 1857 → 1858
(one import line) with justification, per the ratchet's documented raise
path.

Not exercised live: a production MCP key against a real un-enrolled
account (no credentials here). The fix point is the shared server-side
decision helper driven directly by the tests.

# Out of scope, named

- `should_use_legacy_fallback` properties on the two result dataclasses
remain for API compatibility; the routes now use the shared helper
instead.
- Chat/omi_chat consumer surfaces keep their existing rollout semantics
— different consumer contract, untouched.

<!-- This is an auto-generated description by cubic. -->
<a
href="https://cubic.dev/pr/BasedHardware/omi/pull/10095?utm_source=github"
target="_blank" rel="noopener noreferrer"
data-no-image-dialog="true"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img
alt="Review in cubic"
src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a>
<!-- End of auto-generated description by cubic. -->
undivisible added a commit that referenced this pull request Jul 21, 2026
#10097)

# Summary

- `PATCH /v1/users/language` now derives `single_language_mode` from the
live STT capability policy
(`config.stt_provider_policy.supports_live_multilingual_mode`) instead
of the retired Deepgram Nova-3 multi-language list.
- The client mirror (`app/lib/providers/home_provider.dart`) is updated
to the same base-code set with the same regional-variant normalization,
and documents which policy file it mirrors.
- Users whose language the live Modulate auto-detection supports but the
old 19-locale Deepgram list did not — Vietnamese, Korean, Turkish,
Arabic, Thai, and ~40 others — can now actually enter live
multi-language mode.

Fixes #10022.

# Root cause

Two stale copies of a capability decision. #10023 moved live
multi-language capability to the policy boundary
(`supports_live_multilingual_mode`, consumed by the serving path in
`utils/stt/streaming.py`), but the *preference gate* — the route that
decides `single_language_mode` when a user picks a language — still
consulted `deepgram_nova3_multi_languages`, as did its client-side
mirror. A supported user was silently locked into single-language mode
at preference-set time, before serving policy ever ran.

# Failure class

Failure-Class: none

The nearest theme is again divergent read-side policy copies rather than
mutable-state transition ownership (FC-split-mutation-authority's scope)
— same call as #10094/#10095.

# What stays untouched, and why

- `utils/stt/streaming.py`'s Deepgram runtime branch keeps
`deepgram_nova3_multi_languages`: that code selects Deepgram serving
parameters, the one place the Deepgram-specific vocabulary legitimately
lives.
- `migrations/006_auto_set_transcription_mode.py` is a historical
migration; rewriting it would change what a past migration meant.

# Product invariants affected

none (verified with `scripts/pr-preflight --suggest`)

# Validation

- New coverage extends `tests/unit/test_stt_provider_policy.py` (the
guard surface for this policy boundary): behavioral checks that the live
policy admits the newly eligible languages (`vi`, `vi-VN`, `ko`, `tr`,
`ar`, `th`, `pt-BR`, `en`) and rejects unsupported ones (`my`, `am`,
`lo`), plus a **labeled static tripwire** that the route derives
`single_language_mode` from `supports_live_multilingual_mode` and that
`deepgram_nova3_multi_languages` no longer appears in the route module.
The tripwire fails on pre-fix code by construction.
- The route wiring itself is one line; a full route-handler harness for
`routers/users.py`'s heavy import graph would have required module-scope
`sys.modules` stubbing, which the backend-module-isolation checker bans
outside the frozen legacy allowlist — hence policy-seam behavioral tests
+ tripwire, per the repo's testing guidance.
- Repo runner (file-isolated, as CI executes):
`tests/unit/test_stt_provider_policy.py` → 20 passed.
- Dart unit test (`app/test/unit/multilingual_eligibility_test.dart`)
covers the client mirror's eligibility set and normalization. **Not run
locally — this machine has no Flutter SDK**; the test is hermetic and
the app CI lane discovers `app/test/unit/`. Backend coverage of the same
decision did run locally.
- `make preflight` (local lane, this PR body) → all selected checks
pass.
- `black` clean on changed backend files.

Not exercised live: the full app→backend language-selection flow on a
device (no Flutter toolchain here). The behavior change is the
server-side gate, covered by the policy-seam tests and tripwire above.

# Out of scope, named

- The duplicated client list still exists by necessity (Dart cannot
import the Python policy); a follow-up could serve eligibility from the
backend response instead of mirroring the set. The route response
already returns `single_language_mode`, so clients that trust the server
need no list at all — the mirror only affects optimistic local state.

<!-- This is an auto-generated description by cubic. -->
<a
href="https://cubic.dev/pr/BasedHardware/omi/pull/10097?utm_source=github"
target="_blank" rel="noopener noreferrer"
data-no-image-dialog="true"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img
alt="Review in cubic"
src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a>
<!-- End of auto-generated description by cubic. -->
aryanorastar added a commit to aryanorastar/omi that referenced this pull request Jul 25, 2026
…BasedHardware#10203)

GET /v1/dev/user/memories/vector/search failed closed with 403
missing_rollout_state for un-enrolled legacy-cohort accounts, while the
sibling read paths recover: the memory list (BasedHardware#9892/BasedHardware#10094) and MCP
(BasedHardware#10095) already serve the legacy `memories` collection for exactly these
accounts. So an affected account could list its memories but not
vector-search them — the remaining "another rollout-state path still
failing closed" cohort BasedHardware#10203 asks about. (GET /v1/dev/user/memories
itself is already fixed, so a user still hitting 403 on the list endpoint
is a deploy-lag symptom, not this code path.)

search_memories_vector even raised 403 on `should_use_legacy_fallback`
(USE_LEGACY_SAFE) — the one signal that explicitly means "serve legacy."

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 and hydrate results, preserving vector-relevance order and
redacting locked content exactly as the list route does. record_fallback
marks the recovery. Any other deny reason keeps the fail-closed 403.

This is the 4th instance of the same cause (a default-read surface failing
closed for an un-enrolled legacy account instead of serving legacy). The
enforceable guard landed here is the updated source tripwire asserting the
vector route recovers on the shared signal; unifying all read surfaces
behind one predicate ripples into the list route's fallthrough + its
tripwire, so it is tracked as a follow-up rather than widened into this
fix.

Verify (.venv, python3.11):
- test_dev_api_canonical_grant_ordering.py 9 passed, incl. 3 new vector
  cases (legacy fallback on missing_rollout_state + USE_LEGACY_SAFE, and
  other-deny-reason still 403); reverting the fix fails the 2 fallback
  cases and keeps the 403 case (mutation-checked).
- 144 passed across every vector-referencing unit test.

Failure-Class: none

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
kodjima33 pushed a commit that referenced this pull request Jul 25, 2026
…#10203) (#10485)

GET /v1/dev/user/memories/vector/search failed closed with 403
missing_rollout_state for un-enrolled legacy-cohort accounts, while the
sibling read paths recover: the memory list (#9892/#10094) and MCP
(#10095) already serve the legacy `memories` collection for exactly these
accounts. So an affected account could list its memories but not
vector-search them — the remaining "another rollout-state path still
failing closed" cohort #10203 asks about. (GET /v1/dev/user/memories
itself is already fixed, so a user still hitting 403 on the list endpoint
is a deploy-lag symptom, not this code path.)

search_memories_vector even raised 403 on `should_use_legacy_fallback`
(USE_LEGACY_SAFE) — the one signal that explicitly means "serve legacy."

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 and hydrate results, preserving vector-relevance order and
redacting locked content exactly as the list route does. record_fallback
marks the recovery. Any other deny reason keeps the fail-closed 403.

This is the 4th instance of the same cause (a default-read surface failing
closed for an un-enrolled legacy account instead of serving legacy). The
enforceable guard landed here is the updated source tripwire asserting the
vector route recovers on the shared signal; unifying all read surfaces
behind one predicate ripples into the list route's fallthrough + its
tripwire, so it is tracked as a follow-up rather than widened into this
fix.

Verify (.venv, python3.11):
- test_dev_api_canonical_grant_ordering.py 9 passed, incl. 3 new vector
  cases (legacy fallback on missing_rollout_state + USE_LEGACY_SAFE, and
  other-deny-reason still 403); reverting the fix fails the 2 fallback
  cases and keeps the 403 case (mutation-checked).
- 144 passed across every vector-referencing unit test.

Failure-Class: none

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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.

Developer API: GET /v1/dev/user/memories returns 403 missing_rollout_state (conversations/action-items OK)

2 participants