---
number: 10128
title: 'feat(ai): JWT-claim-driven multi-tenant isolation for Memory Core (#10000)'
author: tobiu
state: MERGED
createdAt: '2026-04-20T14:12:32Z'
updatedAt: '2026-04-20T15:08:23Z'
closedAt: '2026-04-20T15:08:19Z'
mergedAt: '2026-04-20T15:08:19Z'
head: agent/10000-jwt-claim-identity
base: dev
url: 'https://github.com/neomjs/neo/pull/10128'
---
## Summary

Establishes end-to-end tenant isolation for the Memory Core. Every ChromaDB write tags the authenticated user's `userId`; every ChromaDB read filters with `where: {userId}`. The identity source of truth is the OIDC-validated Bearer token (via `AuthService`'s introspection response), propagated through the async call chain via a new `RequestContextService` singleton wrapping Node.js `AsyncLocalStorage`. In stdio transport mode no request context is active — writes skip the tag, reads skip the filter, legacy single-tenant behavior preserved.

Core safety claim: two users with distinct `userId` claims writing into the same `sessionId` each see only their own memories on subsequent read.

## Pattern deviation from ticket body (Pattern A chosen over Pattern B)

The ticket's task 1 prescribes extracting identity from *"trusted reverse-proxy HTTP headers (e.g., `x-auth-request-preferred-username`)"* — **Pattern B**, trust the reverse proxy. This PR implements **Pattern A**: extract identity from the OIDC token's `preferred_username` / `sub` claim inside `AuthService.verifyAccessToken`, served as the authoritative source.

Rationale: Pattern A is **self-contained defense-in-depth** — Memory Core validates identity from its own OAuth 2.1 source rather than trusting infrastructure configuration. Both paths need the same request-context plumbing, so Pattern A's cost is just the claim extraction in the verifier (~10 lines). Documented in `RequestContextService` + `AuthService` JSDoc so future operators understand the trust model.

Deployments behind `oauth2-proxy` still work — `oauth2-proxy` forwards the Bearer token, MC validates it independently, Memory Core's claim extraction wins. Both layers coexist; no conflict.

## Scope

| File | Change |
|---|---|
| `ai/mcp/server/shared/services/AuthService.mjs` | Extend `verifyAccessToken` to extract `userId` (`preferred_username` \|\| `sub`) and `username` (`preferred_username` \|\| `name` \|\| `email` \|\| `sub`) from introspection response. Expanded class Anchor documents the identity-propagation contract. |
| `ai/mcp/server/shared/services/RequestContextService.mjs` | **New.** Neo singleton wrapping Node `AsyncLocalStorage`. Exposes `run(context, callback)`, `get()`, `getUserId()`, `getUsername()`. Cross-cutting identity propagation without polluting service signatures. |
| `ai/mcp/server/shared/services/TransportService.mjs` | Wrap `app.all('/mcp')` handler in `RequestContextService.run({userId, username}, () => transport.handleRequest(...))`. Bridges Express `req.auth` context to the MCP dispatch async chain. |
| `ai/mcp/server/memory-core/services/MemoryService.mjs` | `addMemory` tags `metadata.userId`. `listMemories`, `queryMemories`, `getContextFrontier`, `preBriefSession` apply `where: {userId}` — the graph-bridged summary fetches in the last two close a defense-in-depth gap until #10011 isolates the SQLite graph itself. |
| `ai/mcp/server/memory-core/services/SummaryService.mjs` | `listSummaries`, `querySummaries` filter by `userId`. `deleteAllSummaries` switches from global `collection.drop()` to tenant-scoped `collection.delete({where: {userId}})` in cloud mode — prevents one tenant wiping another's data. stdio mode retains legacy drop+recreate behavior. |
| `ai/mcp/server/memory-core/services/SessionService.mjs` | Summary upsert + Antigravity implementation-plan upsert both tag `metadata.userId` when a request context is active. |
| `test/playwright/unit/ai/mcp/server/shared/services/RequestContextService.spec.mjs` | **New.** 6 unit tests covering: default-undefined posture, `run()` scoping, no leak after `run()`, async-boundary survival, nested-context unwind, concurrent-isolation guarantee. |
| `test/playwright/unit/ai/mcp/server/memory-core/services/MemoryService.TenantIsolation.spec.mjs` | **New.** 6 integration tests with a spy-collection stub of `StorageRouter.getMemoryCollection`. Asserts write tags + read filters for both cloud (with context) and stdio (no context) paths. Includes the end-to-end cross-tenant assertion (alice + bob write to same `sessionId`, each reads back only their own). |

## Architectural reality captured in JSDoc

- `RequestContextService` class Anchor: identity flow diagram from `AuthService.verifyAccessToken` → `TransportService.run` → service consumption, with the explicit stdio-mode fallback contract
- `AuthService` class Anchor: extended with the Identity Propagation section cross-referencing `RequestContextService`
- `MemoryService` + `SummaryService` class Anchors: tenant-isolation section noting the #10010 team-vs-private toggle that will later ride on top
- Inline comments at every tag/filter call site referencing ticket #10000 and the substrate (`AsyncLocalStorage` / stdio fallback / defense-in-depth rationale)

## Test plan

- [x] `RequestContextService.spec.mjs` — 6/6 passing in isolation
- [x] `MemoryService.TenantIsolation.spec.mjs` — 6/6 passing in isolation (including the cross-tenant alice-vs-bob assertion)
- [x] Full `memory-core/` + `shared/` suite — 33 passed, 11 failed; all 11 failures reproduce on baseline or pass in isolation (see below)

### Baseline failure accounting

Pre-existing suite failures carried over unchanged from the #10001 → #10007 → #10124 chain:

- 2 × description-length / path-pattern asserts (`McpServerToolLimits`, `FileSystemIngestor`)
- 5 × `SessionSummarization` failing on `aiConfig.engines.neo.*` — tracked in #10126
- 4 × `fullyParallel` interleaving flake in `GraphService.spec` + `QueryReRanker.spec` — 15/15 pass when the two specs run in isolation, confirming the flake is cross-singleton state leakage (tracked by the `symmetric_spec_cleanup` feedback memory)

Net: **zero new failures introduced by this PR**.

## Backward compatibility

- **stdio transport** (local agent dev workflow): no middleware runs, `RequestContextService.getUserId()` returns `undefined`, every tag + filter call site degrades to legacy unfiltered single-tenant behavior. Explicit `stdio fallback` comment at each call site. Existing `SessionSummarization` + concurrent-collection tests (when they otherwise pass) continue unchanged.
- **SSE transport without OIDC**: no `AuthService.setup`, no `requireBearerAuth` middleware, `req.auth` is `undefined`, context object has `undefined` userId — identical fallback.
- **SSE transport with OIDC**: the only mode where isolation activates. Identity flows from claim → context → service.

## Out of scope (separate tickets)

- **#10010 team-vs-private read flag** — operator-facing toggle that keeps writes tagged but skips the read filter. Rides cleanly on top of this PR — just gates the `where.userId` assignment.
- **#10011 Native Edge Graph row-level security** — the `getContextFrontier` / `preBriefSession` defense-in-depth added here is a *mitigation* until #10011 lands the real isolation at the graph layer. Documented inline.
- **Exhaustive per-service integration tests** beyond `MemoryService.TenantIsolation.spec.mjs`. Equivalent spy-collection pattern extends naturally to `SummaryService` and `SessionService`; filing as a polish follow-up rather than bloating this PR's reviewable surface.
- **#10126 engines.neo fix** and the other pre-existing baseline failures. Not this PR's concern.

## What this unblocks

Combined with #10001 (PR #10121) + #10007 (PR #10123) + #10124 (scrub), the multi-tenant Memory Core cloud deployment target now has:

1. Topology toggle (`NEO_CHROMA_UNIFIED`, #10001)
2. Lifecycle bypass in unified mode (#10007)
3. **Identity propagation + per-tenant ChromaDB isolation (this PR, #10000)**

The remaining hard blocker for the deployment is #10010 (team-vs-private read-flag toggle); an engineering team wanting team-shared memory needs that switch to skip the default-deny read filter while keeping writes tagged.

Resolves #10000

Origin Session ID: 1c001810-be28-4554-bb56-c98f9b91bbfb

## Comments

### `@tobiu` commented on 2026-04-20T14:38:14Z

**Input from Claude Opus 4.7 (Claude Code):**

> ❋ ## PR Review Summary
> 
> **Status:** Approved
> 
> **Self-Review Opening:** Self-review of #10000. This implementation chose **Pattern A** (OIDC-claim-driven identity) over the ticket's Pattern B (reverse-proxy header), established a new shared primitive (`RequestContextService` via `AsyncLocalStorage`), and shipped the full write-tag + read-filter + tenant-safe-delete surface as one cohesive PR rather than splitting to avoid a tagged-but-unprotected intermediate state. Key trade-offs and gaps noted below — harsher on Execution-Quality for an identified test-coverage gap in the SummaryService + SessionService write paths.
> 
> ---
> 
> ### 📊 Evaluation Metrics
> 
> *   **`[ARCH_ALIGNMENT]`**: 92 — Shared concern extracted to `shared/services/` (correct namespace). `AsyncLocalStorage` is the standard Node primitive for this, chosen over explicit parameter threading to avoid polluting service signatures. Dual-point defense-in-depth (automatic `TransportService` middleware + method-level `RequestContextService.getUserId()` at every service call site) repeats the #10007 pattern. Minor: the defense-in-depth filters in `getContextFrontier` / `preBriefSession` silently reduce cross-tenant leaks to zero-row returns without a forensic log signal — a `logger.debug` when `userId` is set AND zero docs returned would help operators diagnose tenant misattribution. Polish.
> *   **`[CONTENT_COMPLETENESS]`**: 93 — Class Anchors on all 6 modified services carry a Multi-tenant isolation section cross-referencing `RequestContextService` and ticket #10000. The new `RequestContextService` has the richest Anchor in the PR — identity flow diagram, rationale for `AsyncLocalStorage` vs parameter threading, rationale for OIDC claims vs reverse-proxy header, stdio-mode fallback contract. Inline comments at every tag/filter call site reference #10000 and name the substrate. Deferred: `learn/agentos/MemoryCore.md` docs pass (same rationale as #10001/#10007 — wait for the full multi-tenant story including #10010 before the prose update); `learn/agentos/Authorization.md` could reference the new `userId` / `username` verifier-return fields — small operator-doc gap.
> *   **`[EXECUTION_QUALITY]`**: 85 — 12/12 new tests pass. The cross-tenant assertion in `MemoryService.TenantIsolation.spec.mjs` (alice writes 2, bob writes 1, each reads back only own) is the load-bearing security claim and it's covered explicitly. Symmetric `beforeEach`/`afterEach` restore discipline. **Gap**: the tenant-scoped `SummaryService.deleteAllSummaries` cloud-mode path has no explicit test — it's a destructive + new behavior and deserves its own coverage. Same spy-collection pattern extends trivially. Likewise `SessionService.summarizeSessions` userId tagging. Out-of-scope note in PR body is honest but these are the two call sites where absence of test coverage actually matters.
> *   **`[PRODUCTIVITY]`**: 96 — All 3 ticket tasks delivered plus one security bonus (tenant-scoped `deleteAllSummaries` — the ticket didn't explicitly name this, but leaving it at `collection.drop()` would have let any tenant wipe the whole multi-user deployment). Pattern deviation (A vs B) is documented with rationale; no secret scope cuts.
> *   **`[IMPACT]`**: 85 — This PR is **the** multi-tenant enabler. Without it, the #10001 + #10007 topology work is just "co-located KB+MC on one box with no isolation." Strategic impact is high; immediate runtime impact is partially gated by #10010 (a team wanting shared-team-reads needs that toggle before the full deployment story is complete).
> *   **`[COMPLEXITY]`**: 60 — 8 files, +623 / −51, one new shared primitive (`RequestContextService`), cross-cutting concern threaded through 3 services. Substantial surface area but well-organized — the dual-point defense pattern from #10007 + Anchor & Echo JSDoc make the flow traceable.
> *   **`[EFFORT_PROFILE]`**: Architectural Pillar — introduces new shared infrastructure that future cross-cutting concerns (rate limiting, request-scoped audit logging, tenant-scoped feature flags) can compose onto. Delivers the load-bearing security guarantee for the multi-tenant deployment target. Establishes the substrate for #10010's read-flag toggle.
> 
> ---
> 
> ### 🕸️ Context & Graph Linking
> 
> *   **Target Epic / Issue ID:** Resolves #10000
> *   **Related Graph Nodes:** Epic #9999 (Cloud-Native Knowledge & Multi-Tenant Memory Core); sub-epic #10016 (Multi-Tenant Identity & Data Privacy) — this PR is the first landed sub-ticket in that sub-epic; sub-epic #10015 (Dynamic Topology — complete via #10001 + #10007); upcoming sub-ticket #10010 (team-vs-private read flag) and #10011 (SQLite RLS); defense-in-depth filter in `getContextFrontier` / `preBriefSession` is an explicit mitigation until #10011 lands
> 
> ---
> 
> ### 🧠 Graph Ingestion Notes
> 
> *   **`[TOOLING_GAP]`**: `AsyncLocalStorage`-based integration tests require careful `beforeEach`/`afterEach` restoration to avoid contaminating sibling specs via the shared singleton. The symmetric-cleanup pattern per `feedback_symmetric_spec_cleanup.md` works but duplicates setup boilerplate across each new tenant-isolation spec. A test helper (e.g. `withRequestContext(ctx, fn)` plus a `withSpyCollection(service)` factory) would reduce duplication as more services gain tenant-isolation coverage. Minor; worth filing if the pattern spreads to three or more specs.
> 
> *   **`[RETROSPECTIVE]`**: Three patterns worth permanent capture:
> 
>     1. **`AsyncLocalStorage` for cross-cutting request state.** Standard Node.js primitive that was underused in the codebase before this PR. `RequestContextService` establishes the canonical pattern — future cross-cutting concerns (rate limiting per userId, request-scoped audit logging, tenant-scoped feature flags, trace IDs) can compose onto the same context object without churn at each service call site. Documented contract: middleware sets, services read, stdio-mode fallback via `undefined`.
> 
>     2. **Dual-point defense-in-depth generalizes across infrastructure-boundary features.** #10007 established this at the lifecycle boundary (`initAsync` + `startDatabase` both guarded); #10000 repeats at the identity boundary (`TransportService` middleware + every service-level tag/filter call site). Emerging rule: **when a feature has both an automatic entry point AND a manual MCP-tool-exposed entry point, guard at BOTH layers**. Encodes the lesson that automatic-only guards miss operator-initiated paths; method-level-only guards miss race conditions during init.
> 
>     3. **Quality-over-speed recalibration catches architectural gaps pre-implementation.** The initial reflex was Pattern B (ticket's prescription) + write-side-only split. The pause-to-analyze discipline caught: (a) Pattern B trusts infrastructure when Pattern A has the same plumbing cost with self-contained validation, (b) a write-tag-without-read-filter split would create a tagged-but-unprotected intermediate state (data classified but not isolated — strictly worse than current baseline), (c) cohesive single PR is cleaner despite larger diff. **Generalization: pause-to-analyze the prescribed prescription when "quick win" reflex is fighting with "this changes the architecture" reality.** Encoded as the `feedback_quality_over_speed.md` memory saved this session.
> 
> ---
> 
> ### 📋 Required Actions
> 
> No blocking changes. Three optional items — two polish commits worth pushing pre-merge if you want them locked in, one deferred-doc follow-up.
> 
> - [ ] *(Polish)* Add explicit test coverage for `SummaryService.deleteAllSummaries` tenant-scoped cloud-mode path. Destructive + new behavior; deserves its own assertion. Same spy-collection pattern extends naturally.
> - [ ] *(Polish)* Add explicit test coverage for `SessionService.summarizeSessions` userId tagging on the summary upsert + Antigravity plan upsert. Write-path-specific.
> - [ ] *(Follow-up, not this PR)* `learn/agentos/Authorization.md` — reference the new `userId` / `username` verifier-return fields so operators configuring OIDC know what downstream services can read. Can land alongside the `MemoryCore.md` multi-tenant docs pass when #10010 completes.
> 
> Approved for merge. Next in sequence: #10010 (team-vs-private read flag) to complete the multi-tenant deployment path, then tobi's call on whether to file the three polish items as a follow-up or push them here.

---

### `@tobiu` commented on 2026-04-20T15:06:51Z

**Input from Claude Opus 4.7 (Claude Code):**

> ❋ Pre-polish safety work + partial polish landed per @tobiu's direction.
> 
> ### Safety backup before touching destructive paths
> 
> Ran `node buildScripts/ai/exportDatabase.mjs` before any test work. Exported **8191 memories + 794 summaries + 15 graph elements** to `.neo-ai-data/backups/`. Full re-summarization would have been the recovery cost if a spec error had nuked production data — so the backup is the load-bearing safety net behind this polish pass.
> 
> ### Follow-up ticket filed
> 
> [#10129](https://github.com/neomjs/neo/issues/10129) — `exportDatabase.mjs: route via services.mjs SDK + cover Knowledge Base`. The current script has two gaps surfaced by the backup run: direct `ai/mcp/server/...` imports bypass the `ai/services.mjs` Zod-validated SDK boundary, and only Memory Core is covered — Knowledge Base has no `manageDatabaseBackup` surface at all. Two-phase implementation plan documented in the ticket.
> 
> ### `78b13a300` — SummaryService tenant-isolation spec
> 
> New `test/playwright/unit/ai/mcp/server/memory-core/services/SummaryService.TenantIsolation.spec.mjs` — 6 tests, spy-collection pattern, **zero real-DB contact** (explicitly called out in the spec's top JSDoc). Covers:
> 
> - `deleteAllSummaries` cloud-mode scopes to active tenant — **the load-bearing assertion: `collection.drop()` is NOT called, `collection.delete({where: {userId}})` is**. Seeded with 2 Alice + 3 Bob summaries; Alice's delete leaves all 3 of Bob's intact.
> - `deleteAllSummaries` stdio-mode preserves legacy `drop()` + recreate path (single-tenant backward-compat).
> - `deleteAllSummaries` cloud-mode with zero tenant rows skips the delete call entirely (no-op round-trip elimination).
> - `listSummaries` filters by `userId` from request context.
> - `querySummaries` merges `userId` with caller-provided `category` filter.
> - `querySummaries` without a request context leaves the caller-provided where as-is.
> 
> ### SessionService polish — deferred
> 
> The `SessionService.summarizeSessions` + `ingestAntigravityArtifacts` userId-tagging test requires mocking `this.memoryCollection` + `this.sessionsCollection` + `this.model.generateContent` + `GraphService` + `fs.readdirSync` / `fs.readFileSync` — **substantial test-machinery scope** that would bloat this polish commit. Cleaner path is a small refactor extracting the metadata-construction into a pure `buildSummaryMetadata(...)` / `buildPlanMetadata(...)` helper, then unit-testing the helper directly.
> 
> That refactor adds real value (testability + reuse) beyond just enabling the test, so it belongs as its own ticket rather than inline in this PR. Happy to file if you want it queued; flagging rather than auto-filing since it's borderline between "legitimate follow-up" and "test-coverage-only scope" and your call wins.
> 
> ### `Authorization.md` doc reference
> 
> Deferred to the consolidated `MemoryCore.md` + `Authorization.md` multi-tenant docs pass that rides alongside #10010's completion — same rationale as the #10001 / #10007 doc deferrals (avoid partial-feature prose drift).
> 
> ### Cumulative state
> 
> Two commits on PR:
> - `8265e3be5` — feat(ai): JWT-claim-driven multi-tenant isolation for Memory Core (#10000)
> - `78b13a300` — test(ai): add tenant isolation spec for SummaryService (#10000)
> 
> 18 tests total passing (6 RequestContextService + 6 MemoryService.TenantIsolation + 6 SummaryService.TenantIsolation). Zero new suite failures.

---

