Skip to content

Commit 1790baf

Browse files
Muguett-DBYMuguett-DBY
committed
fix(client): resume MCP catalogs after location rebuilds
1 parent ab9bf4a commit 1790baf

2 files changed

Lines changed: 53 additions & 143 deletions

File tree

‎packages/client/src/solid/data.ts‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,7 @@ export function createData(config: CreateDataInput) {
284284
)
285285
const messageIndex = new Map<string, Map<string, number>>()
286286
const sync = createSync()
287-
// Teardown publishes MCP events after shutdown; a later read reactivates the location.
287+
// MCP teardown events must not reopen a shut-down location.
288288
const shutdowns = new Set<string>()
289289
let activeUpdates: Map<string, DataSessionStatus | undefined> | undefined
290290
const pendingUpdates = new Map<string, Map<string, SessionInboxInfo | SessionInbox.Delivery | undefined>>()
@@ -1264,6 +1264,26 @@ export function createData(config: CreateDataInput) {
12641264

12651265
if (!event.location) return
12661266
const location = event.location
1267+
// Catalog updates come from a live location. Its rebuilt MCP events may have arrived first.
1268+
switch (event.type) {
1269+
case "provider.updated":
1270+
case "model.updated":
1271+
case "agent.updated":
1272+
case "command.updated":
1273+
case "skill.updated":
1274+
case "reference.updated":
1275+
case "integration.updated":
1276+
case "config.updated":
1277+
case "websearch.updated": {
1278+
const key = locationKey(location)
1279+
if (!shutdowns.delete(key)) break
1280+
if (result.location.mcp.server.list(location) !== undefined || sync.has(`location.mcpServer:${key}`))
1281+
refresh(() => result.location.mcp.server.sync(location))
1282+
if (result.location.mcp.resource.list(location) !== undefined || sync.has(`location.mcpResource:${key}`))
1283+
refresh(() => result.location.mcp.resource.sync(location))
1284+
break
1285+
}
1286+
}
12671287
switch (event.type) {
12681288
case "location.shutdown": {
12691289
const key = locationKey(location)

‎packages/client/test/solid-data.test.ts‎

Lines changed: 32 additions & 142 deletions
Original file line numberDiff line numberDiff line change
@@ -1208,186 +1208,73 @@ test.each(mcpEvents)("does not load unread MCP catalogs on %s", async (type) =>
12081208
}
12091209
})
12101210

1211-
test.each(mcpEvents)("refreshes a loaded MCP catalog on %s", async (type) => {
1212-
const setup = mcpFixture()
1213-
try {
1214-
await setup.load("/project")
1215-
setup.requests.length = 0
1216-
for (let index = 0; index < (type === "mcp.status.changed" ? 9 : 1); index++) setup.emit(type)
1217-
await setup.flush()
1218-
expect(setup.requests).toEqual([`${type === "mcp.status.changed" ? "/api/mcp" : "/api/mcp/resource"}:/project`])
1219-
} finally {
1220-
setup.dispose()
1221-
}
1222-
})
1223-
1224-
test("does not load an unread resource catalog just because its server catalog is loaded", async () => {
1225-
const setup = mcpFixture()
1226-
try {
1227-
await setup.data.location.mcp.server.sync()
1228-
setup.requests.length = 0
1229-
setup.emit("mcp.resources.changed")
1230-
await setup.flush()
1231-
expect(setup.requests).toEqual([])
1232-
} finally {
1233-
setup.dispose()
1234-
}
1235-
})
1236-
1237-
test("keeps a shut-down location idle through MCP teardown events", async () => {
1238-
const setup = mcpFixture()
1239-
try {
1240-
await setup.load("/project")
1241-
setup.requests.length = 0
1242-
setup.emit("location.shutdown")
1243-
for (const type of mcpEvents) setup.emit(type)
1244-
await setup.flush()
1245-
expect(setup.requests).toEqual([])
1246-
} finally {
1247-
setup.dispose()
1248-
}
1249-
})
1250-
1251-
test("cancels a queued MCP status refresh when its location shuts down", async () => {
1211+
test.each(mcpEvents)("keeps teardown %s idle and resumes on an explicit read", async (type) => {
12521212
const setup = mcpFixture()
1213+
const path = type === "mcp.status.changed" ? "/api/mcp" : "/api/mcp/resource"
12531214
try {
12541215
await setup.load("/project")
1216+
await setup.load("/other")
12551217
setup.requests.length = 0
1256-
setup.emit("mcp.status.changed")
12571218
setup.emit("location.shutdown")
1219+
for (const event of mcpEvents) setup.emit(event)
1220+
setup.emit(type, "/other")
12581221
await setup.flush()
1259-
expect(setup.requests).toEqual([])
1260-
} finally {
1261-
setup.dispose()
1262-
}
1263-
})
1264-
1265-
test("resumes MCP reads and events after an explicit location read", async () => {
1266-
const setup = mcpFixture()
1267-
try {
1268-
await setup.load("/project")
1269-
setup.emit("location.shutdown")
1222+
expect(setup.requests).toEqual([`${path}:/other`])
12701223
setup.requests.length = 0
12711224
await setup.load("/project")
12721225
expect(setup.requests.toSorted()).toEqual(["/api/mcp/resource:/project", "/api/mcp:/project"])
12731226
setup.requests.length = 0
1274-
for (const type of mcpEvents) setup.emit(type)
1275-
await setup.flush()
1276-
expect(setup.requests.toSorted()).toEqual(["/api/mcp/resource:/project", "/api/mcp:/project"])
1277-
} finally {
1278-
setup.dispose()
1279-
}
1280-
})
1281-
1282-
test("keeps other locations' MCP refreshes active after a location shuts down", async () => {
1283-
const setup = mcpFixture()
1284-
try {
1285-
await setup.load("/project")
1286-
await setup.load("/other")
1287-
setup.requests.length = 0
1288-
setup.emit("location.shutdown")
1289-
for (const type of mcpEvents) {
1290-
setup.emit(type)
1291-
setup.emit(type, "/other")
1292-
}
1227+
setup.emit(type)
12931228
await setup.flush()
1294-
expect(setup.requests.toSorted()).toEqual(["/api/mcp/resource:/other", "/api/mcp:/other"])
1229+
expect(setup.requests).toEqual([`${path}:/project`])
12951230
} finally {
12961231
setup.dispose()
12971232
}
12981233
})
12991234

1300-
test("resumes cached MCP events when the location info is explicitly reopened", async () => {
1235+
test.each([
1236+
["provider before MCP", "provider.updated", false],
1237+
["provider after MCP", "provider.updated", true],
1238+
["agent before MCP", "agent.updated", false],
1239+
["agent after MCP", "agent.updated", true],
1240+
] as const)("refreshes rebuilt MCP catalogs: %s", async (_name, type, mcpFirst) => {
13011241
const setup = mcpFixture()
13021242
try {
13031243
await setup.load("/project")
13041244
setup.emit("location.shutdown")
1305-
await setup.data.location.syncInfo()
13061245
setup.requests.length = 0
1307-
for (const type of mcpEvents) setup.emit(type)
1308-
await setup.flush()
1309-
expect(setup.requests.toSorted()).toEqual(["/api/mcp/resource:/project", "/api/mcp:/project"])
1310-
} finally {
1311-
setup.dispose()
1312-
}
1313-
})
1314-
1315-
test("does not reactivate a location when an older MCP read finishes after shutdown", async () => {
1316-
const requested = Promise.withResolvers<void>()
1317-
const release = Promise.withResolvers<void>()
1318-
const setup = mcpFixture(async () => {
1319-
requested.resolve()
1320-
await release.promise
1321-
})
1322-
const load = setup.data.location.mcp.resource.sync()
1323-
try {
1324-
await requested.promise
1325-
setup.emit("location.shutdown")
1326-
release.resolve()
1327-
await load
1328-
setup.requests.length = 0
1329-
for (const type of mcpEvents) setup.emit(type)
1246+
if (mcpFirst) for (const event of mcpEvents) setup.emit(event)
1247+
setup.emit(type)
1248+
if (!mcpFirst) for (const event of mcpEvents) setup.emit(event)
13301249
await setup.flush()
1331-
expect(setup.requests).toEqual([])
1250+
expect(new Set(setup.requests.filter((request) => request.startsWith("/api/mcp")))).toEqual(
1251+
new Set(["/api/mcp:/project", "/api/mcp/resource:/project"]),
1252+
)
13321253
} finally {
1333-
release.resolve()
1334-
await load
13351254
setup.dispose()
13361255
}
13371256
})
13381257

1339-
test.each(mcpEvents)("does not run a queued MCP reload after shutdown (%s)", async (type) => {
1258+
test("discards queued MCP refreshes and ignores an old read finishing after shutdown", async () => {
13401259
const requested = Promise.withResolvers<void>()
13411260
const release = Promise.withResolvers<void>()
1342-
let reads = 0
1343-
const setup = mcpFixture(async () => {
1344-
if (++reads !== 1) return
1261+
const setup = mcpFixture(async (url) => {
1262+
if (url.pathname !== "/api/mcp/resource") return
13451263
requested.resolve()
13461264
await release.promise
13471265
})
1348-
const catalog = type === "mcp.status.changed" ? setup.data.location.mcp.server : setup.data.location.mcp.resource
1349-
const path = type === "mcp.status.changed" ? "/api/mcp" : "/api/mcp/resource"
1350-
const load = catalog.sync()
1266+
const load = setup.load("/project")
13511267
try {
13521268
await requested.promise
1353-
setup.emit(type)
1354-
await setup.flush()
1355-
setup.emit("location.shutdown")
1356-
release.resolve()
1357-
await load
13581269
await setup.flush()
1359-
expect(setup.requests).toEqual([`${path}:/project`])
1360-
await catalog.sync()
1361-
expect(setup.requests).toEqual([`${path}:/project`, `${path}:/project`])
1362-
} finally {
1363-
release.resolve()
1364-
await load
1365-
setup.dispose()
1366-
}
1367-
})
1368-
1369-
test("revalidates when an MCP event overtakes the first catalog read", async () => {
1370-
const requested = Promise.withResolvers<void>()
1371-
const release = Promise.withResolvers<void>()
1372-
const refreshed = Promise.withResolvers<void>()
1373-
let reads = 0
1374-
const setup = mcpFixture(async () => {
1375-
if (++reads === 1) {
1376-
requested.resolve()
1377-
await release.promise
1378-
return
1379-
}
1380-
refreshed.resolve()
1381-
})
1382-
const load = setup.data.location.mcp.resource.sync()
1383-
try {
1384-
await requested.promise
13851270
setup.emit("mcp.resources.changed")
1271+
setup.emit("mcp.status.changed")
1272+
setup.emit("location.shutdown")
13861273
release.resolve()
13871274
await load
1388-
await refreshed.promise
1275+
for (const type of mcpEvents) setup.emit(type)
13891276
await setup.flush()
1390-
expect(setup.requests).toEqual(["/api/mcp/resource:/project", "/api/mcp/resource:/project"])
1277+
expect(setup.requests.toSorted()).toEqual(["/api/mcp/resource:/project", "/api/mcp:/project"])
13911278
} finally {
13921279
release.resolve()
13931280
await load
@@ -1432,7 +1319,10 @@ function mcpFixture(read?: (url: URL) => Promise<void>) {
14321319
requests,
14331320
load: (directory: string) =>
14341321
Promise.all([data.location.mcp.server.sync({ directory }), data.location.mcp.resource.sync({ directory })]),
1435-
emit(type: (typeof mcpEvents)[number] | "location.shutdown", directory = "/project") {
1322+
emit(
1323+
type: (typeof mcpEvents)[number] | "location.shutdown" | "provider.updated" | "agent.updated",
1324+
directory = "/project",
1325+
) {
14361326
const details: OpenCodeEvent = {
14371327
id: `evt_mcp_${++sequence}`,
14381328
created: sequence,

0 commit comments

Comments
 (0)