Skip to content

Commit 4eab26a

Browse files
committed
fix(mcp): invalidate stale startup work
1 parent 6a477c4 commit 4eab26a

2 files changed

Lines changed: 75 additions & 84 deletions

File tree

packages/opencode/src/mcp/index.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -610,14 +610,14 @@ export const layer = Layer.effect(
610610

611611
const startConfigured = Effect.fn("MCP.startConfigured")(function* (
612612
s: State,
613-
entries: ReadonlyArray<readonly [string, ConfigMCP.Info]>,
613+
entries: ReadonlyArray<readonly [string, ConfigMCP.Info, number]>,
614614
) {
615615
yield* startupLock.withPermits(1)(
616616
Effect.forEach(
617617
entries,
618-
([key, mcp]) =>
618+
([key, mcp, revision]) =>
619619
Effect.gen(function* () {
620-
const revision = s.revision[key] ?? 0
620+
if ((s.revision[key] ?? 0) !== revision) return
621621
const result = yield* createSafely(key, mcp)
622622
if ((s.revision[key] ?? 0) !== revision) {
623623
yield* closeCreateResult(result)
@@ -654,7 +654,7 @@ export const layer = Layer.effect(
654654
}
655655

656656
s.status[key] = { status: "connecting" }
657-
return [[key, mcp] as const]
657+
return [[key, mcp, bump(s, key)] as const]
658658
})
659659

660660
if (configured.length > 0) {

packages/opencode/test/mcp/lifecycle.test.ts

Lines changed: 71 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,15 @@ function deferred() {
216216
return { promise, resolve }
217217
}
218218

219+
const waitConnected = (mcp: MCPNS.Interface, name: string) =>
220+
pollWithTimeout(
221+
Effect.gen(function* () {
222+
const status = yield* mcp.status()
223+
return status[name]?.status === "connected" ? true : undefined
224+
}),
225+
`configured mcp ${name} did not connect`,
226+
)
227+
219228
it.instance(
220229
"status() returns connecting without waiting for configured startup",
221230
() =>
@@ -255,42 +264,60 @@ it.instance(
255264
},
256265
)
257266

258-
it.live("configured MCP startup runs for one project at a time", () =>
259-
Effect.gen(function* () {
260-
const connect = deferred()
261-
connectHook = () => connect.promise
262-
const config = {
267+
it.instance(
268+
"configured MCP startup runs for one project at a time",
269+
() =>
270+
MCP.Service.use((mcp: MCPNS.Interface) =>
271+
Effect.gen(function* () {
272+
const connect = deferred()
273+
connectHook = () => connect.promise
274+
const config = {
275+
mcp: {
276+
"slow-server": {
277+
type: "local" as const,
278+
command: ["echo", "test"],
279+
},
280+
},
281+
}
282+
const second = yield* tmpdirScoped({ config })
283+
284+
yield* mcp.status()
285+
yield* pollWithTimeout(
286+
Effect.sync(() => (connectStarts === 1 ? true : undefined)),
287+
"first configured mcp startup did not begin",
288+
)
289+
290+
yield* mcp.status().pipe(provideInstance(second))
291+
yield* Effect.sleep("100 millis")
292+
expect(connectStarts).toBe(1)
293+
expect(maxActiveConnects).toBe(1)
294+
295+
connect.resolve()
296+
297+
yield* pollWithTimeout(
298+
Effect.gen(function* () {
299+
const first = yield* mcp.status()
300+
const next = yield* mcp.status().pipe(provideInstance(second))
301+
return first["slow-server"]?.status === "connected" && next["slow-server"]?.status === "connected"
302+
? true
303+
: undefined
304+
}),
305+
"configured mcp startups did not complete",
306+
)
307+
expect(connectStarts).toBe(2)
308+
expect(maxActiveConnects).toBe(1)
309+
}),
310+
).pipe(Effect.provide(CrossSpawnSpawner.defaultLayer)),
311+
{
312+
config: {
263313
mcp: {
264314
"slow-server": {
265-
type: "local" as const,
315+
type: "local",
266316
command: ["echo", "test"],
267317
},
268318
},
269-
}
270-
271-
const first = yield* tmpdirScoped({ config })
272-
const second = yield* tmpdirScoped({ config })
273-
const mcp = yield* MCP.Service
274-
275-
yield* mcp.status().pipe(provideInstance(first))
276-
yield* pollWithTimeout(
277-
Effect.sync(() => (connectStarts === 1 ? true : undefined)),
278-
"first configured mcp startup did not begin",
279-
)
280-
281-
yield* mcp.status().pipe(provideInstance(second))
282-
yield* Effect.sleep("100 millis")
283-
expect(connectStarts).toBe(1)
284-
expect(maxActiveConnects).toBe(1)
285-
286-
connect.resolve()
287-
288-
yield* pollWithTimeout(
289-
Effect.sync(() => (connectStarts === 2 && activeConnects === 0 ? true : undefined)),
290-
"second configured mcp startup did not run after first completed",
291-
)
292-
expect(maxActiveConnects).toBe(1)
293-
}).pipe(Effect.provide(CrossSpawnSpawner.defaultLayer)),
319+
},
320+
},
294321
)
295322

296323
// ========================================================================
@@ -377,10 +404,8 @@ it.instance(
377404
lastCreatedClientName = "disc-server"
378405
getOrCreateClientState("disc-server")
379406

380-
yield* mcp.add("disc-server", {
381-
type: "local",
382-
command: ["echo", "test"],
383-
})
407+
yield* mcp.status()
408+
yield* waitConnected(mcp, "disc-server")
384409

385410
const statusBefore = yield* mcp.status()
386411
expect(statusBefore["disc-server"]?.status).toBe("connected")
@@ -418,10 +443,8 @@ it.instance(
418443
{ name: "my_tool", description: "a tool", inputSchema: { type: "object", properties: {} } },
419444
]
420445

421-
yield* mcp.add("reconn-server", {
422-
type: "local",
423-
command: ["echo", "test"],
424-
})
446+
yield* mcp.status()
447+
yield* waitConnected(mcp, "reconn-server")
425448

426449
yield* mcp.disconnect("reconn-server")
427450
expect((yield* mcp.status())["reconn-server"]?.status).toBe("disabled")
@@ -488,7 +511,7 @@ it.instance(
488511
// ========================================================================
489512

490513
it.instance(
491-
"init connects available servers even when one fails",
514+
"connects available servers even when one fails",
492515
() =>
493516
MCP.Service.use((mcp: MCPNS.Interface) =>
494517
Effect.gen(function* () {
@@ -526,14 +549,8 @@ it.instance(
526549
{
527550
config: {
528551
mcp: {
529-
"good-server": {
530-
type: "local",
531-
command: ["echo", "good"],
532-
},
533-
"bad-server": {
534-
type: "local",
535-
command: ["echo", "bad"],
536-
},
552+
"good-server": { type: "local", command: ["echo", "good"], enabled: false },
553+
"bad-server": { type: "local", command: ["echo", "bad"], enabled: false },
537554
},
538555
},
539556
},
@@ -658,16 +675,7 @@ it.instance(
658675
expect(key).toContain("my-prompt")
659676
}),
660677
),
661-
{
662-
config: {
663-
mcp: {
664-
"prompt-server": {
665-
type: "local",
666-
command: ["echo", "test"],
667-
},
668-
},
669-
},
670-
},
678+
{ config: { mcp: {} } },
671679
)
672680

673681
it.instance(
@@ -691,16 +699,7 @@ it.instance(
691699
expect(key).toContain("my-resource")
692700
}),
693701
),
694-
{
695-
config: {
696-
mcp: {
697-
"resource-server": {
698-
type: "local",
699-
command: ["echo", "test"],
700-
},
701-
},
702-
},
703-
},
702+
{ config: { mcp: {} } },
704703
)
705704

706705
it.instance(
@@ -712,10 +711,8 @@ it.instance(
712711
const serverState = getOrCreateClientState("prompt-disc-server")
713712
serverState.prompts = [{ name: "hidden-prompt", description: "Should not appear" }]
714713

715-
yield* mcp.add("prompt-disc-server", {
716-
type: "local",
717-
command: ["echo", "test"],
718-
})
714+
yield* mcp.status()
715+
yield* waitConnected(mcp, "prompt-disc-server")
719716

720717
yield* mcp.disconnect("prompt-disc-server")
721718

@@ -824,10 +821,7 @@ it.instance(
824821
{
825822
config: {
826823
mcp: {
827-
"fail-connect": {
828-
type: "local",
829-
command: ["echo", "test"],
830-
},
824+
"fail-connect": { type: "local", command: ["echo", "test"], enabled: false },
831825
},
832826
},
833827
},
@@ -895,10 +889,7 @@ it.instance(
895889
{
896890
config: {
897891
mcp: {
898-
"my.special-server": {
899-
type: "local",
900-
command: ["echo", "test"],
901-
},
892+
"my.special-server": { type: "local", command: ["echo", "test"], enabled: false },
902893
},
903894
},
904895
},

0 commit comments

Comments
 (0)