fix(client): scope corrupt-cache-entry deletion to the probed partition

.changeset/partition-scoped-corrupt-delete.md

@@ -0,0 +1,5 @@
+---
+'@modelcontextprotocol/client': patch
+---
+
+A corrupt cached document now deletes only the partition it was read from. The previous cleanup deleted both partitions, so on a shared multi-principal store one principal's corrupt private entry also evicted the healthy shared entry. Store delete failures during this cleanup are reported through the error sink instead of rejecting the read.

packages/client/src/client/responseCache.ts

@@ -382,15 +382,15 @@ export class ClientResponseCache {
      * `cachePartition` is `''` the two partitions are identical and only one
      * probe is issued.
      */
-    private async _probe(method: string, params?: string): Promise<CacheEntry | undefined> {
+    private async _probe(method: string, params?: string): Promise<{ entry: CacheEntry; partition: string } | undefined> {
         const key = { method, params: params ?? '' };
         const ownPartition = this._partitionFor('private');
         const own = await this._store.get({ ...key, partition: ownPartition });
-        if (own !== undefined) return own;
+        if (own !== undefined) return { entry: own, partition: ownPartition };
         const sharedPartition = this._partitionFor('public');
         if (sharedPartition === ownPartition) return undefined;
         const shared = await this._store.get({ ...key, partition: sharedPartition });
-        return shared?.scope === 'public' ? shared : undefined;
+        return shared?.scope === 'public' ? { entry: shared, partition: sharedPartition } : undefined;
     }
 
     /**
@@ -560,17 +560,23 @@ export class ClientResponseCache {
      * `expiresAt` passes.
      */
     async read(method: string, params?: string): Promise<{ value: unknown } | undefined> {
-        const entry = await this._probe(method, params);
-        if (entry?.expiresAt === undefined || !(entry.expiresAt > this.now())) return undefined;
+        const probed = await this._probe(method, params);
+        if (probed?.entry.expiresAt === undefined || !(probed.entry.expiresAt > this.now())) return undefined;
         try {
-            const parsed: unknown = JSON.parse(entry.value);
+            const parsed: unknown = JSON.parse(probed.entry.value);
             if (typeof parsed !== 'object' || parsed === null || Array.isArray(parsed)) {
                 throw new TypeError('cached document is not an object');
             }
             return { value: parsed };
         } catch (error) {
+            // Corruption is evidence about one physical entry; the sibling
+            // partition was never probed and may be a co-tenant's healthy copy.
             this._reportError(error);
-            await this._deleteBoth(method, params ?? '');
+            try {
+                await this._store.delete({ method, params: params ?? '', partition: probed.partition });
+            } catch (deleteError) {
+                this._reportError(deleteError);
+            }
             return undefined;
         }
     }
@@ -604,7 +610,8 @@ export class ClientResponseCache {
      * and, via {@linkcode outputValidator}, its output-schema validation.
      */
     async toolDefinition(name: string): Promise<Tool | undefined> {
-        const entry = await this._probe('tools/list');
+        const probed = await this._probe('tools/list');
+        const entry = probed?.entry;
         if (entry === undefined) {
             this._toolIndex = undefined;
             return undefined;
@@ -644,7 +651,8 @@ export class ClientResponseCache {
      * the lot.
      */
     async outputValidator<V>(name: string, compile: (tool: Tool) => V | undefined): Promise<V | undefined> {
-        const entry = await this._probe('tools/list');
+        const probed = await this._probe('tools/list');
+        const entry = probed?.entry;
         if (entry === undefined) {
             this._toolOutputValidatorIndex = undefined;
             return undefined;

packages/client/test/client/responseCacheCodec.test.ts

@@ -248,6 +248,50 @@ describe('response cache document codec', () => {
         }
     });
 
+    test('a corrupt own-partition entry deletes only the probed partition, not the shared sibling', async () => {
+        const reported: unknown[] = [];
+        const gets: string[] = [];
+        const deletes: string[] = [];
+        const store: ResponseCacheStore = {
+            // Own probe (first get) hits a corrupt entry; the shared partition is never touched.
+            get: key => {
+                gets.push(key.partition ?? '');
+                return { value: '{ not json', stamp: 1, expiresAt: Date.now() + 60_000, scope: 'private' as const };
+            },
+            set: () => 1,
+            delete: key => {
+                deletes.push(key.partition ?? '');
+            },
+            evict: () => {},
+            clear: () => {}
+        };
+        // Non-empty cachePartition: own !== shared, so a two-partition delete
+        // (the pre-fix behavior) would be observable as a second delete.
+        const cache = new ClientResponseCache(store, true, error => reported.push(error), 'alice');
+        expect(await cache.read('tools/list')).toBeUndefined();
+        expect(reported).toHaveLength(1);
+        expect(gets).toHaveLength(1);
+        expect(deletes).toEqual([gets[0]]);
+        expect(gets[0]).toContain('alice');
+    });
+
+    test('a failing store delete during corrupt-entry cleanup is reported, not thrown to the caller', async () => {
+        const reported: unknown[] = [];
+        const store: ResponseCacheStore = {
+            get: () => ({ value: '{ not json', stamp: 1, expiresAt: Date.now() + 60_000, scope: 'private' as const }),
+            set: () => 1,
+            delete: () => {
+                throw new Error('store connection lost');
+            },
+            evict: () => {},
+            clear: () => {}
+        };
+        const cache = new ClientResponseCache(store, true, error => reported.push(error));
+        await expect(cache.read('tools/list')).resolves.toBeUndefined();
+        expect(reported).toHaveLength(2);
+        expect(String(reported[1])).toMatch(/connection lost/);
+    });
+
     test('a tools/list document with non-object elements is reported once per stamp, not thrown per lookup', async () => {
         const reported: unknown[] = [];
         const store: ResponseCacheStore = {