Skip to content

Commit 3e94585

Browse files
cliffhallclaude
andcommitted
Address review notes on resourceSubscriptionsState
- Soften the onResourceUpdated comment: the client's dispatch is already guarded by subscribedResources.has(uri), so the re-check is true defense-in-depth rather than guarding a known hazard. - Use this.getSubscriptions() in the statusChange handler so every emit goes through the defensive-copy path. - Document the deliberate fallback to a synthetic Resource when a previously-listed resource is removed from the managed list while the user is still subscribed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent ac9065f commit 3e94585

1 file changed

Lines changed: 9 additions & 5 deletions

File tree

core/mcp/state/resourceSubscriptionsState.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,10 @@
99
* When no resource is found in the managed list (e.g. a template-expanded URI
1010
* the user subscribed to before the resources list refreshed), a synthetic
1111
* Resource `{ uri, name: uri }` is used — mirroring the fallback pattern in
12-
* ResourcesScreen.
12+
* ResourcesScreen. If the server later removes a previously-listed resource
13+
* while the user is still subscribed, the tile regresses to that synthetic
14+
* form: the managed list is the source of truth, so displaying a stale name
15+
* for a server-removed resource is intentionally avoided.
1316
*/
1417

1518
import type { InspectorClientProtocol } from "../inspectorClientProtocol.js";
@@ -69,9 +72,10 @@ export class ResourceSubscriptionsState extends TypedEventTarget<ResourceSubscri
6972
event: TypedEventGeneric<InspectorClientEventMap, "resourceUpdated">,
7073
): void => {
7174
const { uri } = event.detail;
72-
// Only stamp + emit if the URI is currently subscribed. The client
73-
// already guards on subscribedResources before dispatching, but we
74-
// re-check so out-of-order events can't resurrect a stale entry.
75+
// Belt-and-braces: the client's dispatch site is already guarded by
76+
// subscribedResources.has(uri), so this re-check should be redundant.
77+
// It stays correct if a future change ever decouples dispatch from
78+
// subscription state.
7579
if (!this.subscribedUris.includes(uri)) return;
7680
this.lastUpdatedByUri.set(uri, new Date());
7781
this.rebuild();
@@ -82,7 +86,7 @@ export class ResourceSubscriptionsState extends TypedEventTarget<ResourceSubscri
8286
this.subscribedUris = [];
8387
this.lastUpdatedByUri.clear();
8488
this.subscriptions = [];
85-
this.dispatchTypedEvent("subscriptionsChange", []);
89+
this.dispatchTypedEvent("subscriptionsChange", this.getSubscriptions());
8690
}
8791
};
8892

0 commit comments

Comments
 (0)