fix(mcp): diagnose elicitation-transport CI flake with observability

packages/opencode/src/mcp/elicitation.ts

@@ -43,9 +43,23 @@ const ELICITATION_TIMEOUT_MS = 300_000
  * concurrent cross-session MCP calls are best-effort (last writer wins) —
  * acceptable and strictly better than ALS-only routing, which never resolves.
  */
-let activeSession: string | undefined
-export const setActiveElicitationSession = (id: string | undefined) => {
-  activeSession = id
+type ActiveElicitationSession = { id: string; token: number }
+type ActiveElicitationSessionCleanup = () => void
+
+let activeSessionToken = 0
+const activeSession: ActiveElicitationSession[] = []
+export const setActiveElicitationSession = (id: string | undefined): ActiveElicitationSessionCleanup => {
+  if (id === undefined) {
+    activeSession.splice(0)
+    return () => {}
+  }
+
+  const entry = { id, token: activeSessionToken++ }
+  activeSession.push(entry)
+  return () => {
+    const index = activeSession.findIndex((item) => item.token === entry.token)
+    if (index >= 0) activeSession.splice(index, 1)
+  }
 }
 
 export type ElicitAction = "accept" | "decline" | "cancel"
@@ -239,8 +253,16 @@ export const handleElicitation = Effect.fn("MCP.elicitation.handle")(function* (
   const notification = Option.getOrUndefined(yield* Effect.serviceOption(Notification.Service))
   const sessionID = input.sessionID
 
-  // No interaction layer or no session context → decline immediately.
+  // No interaction layer or no session context → decline immediately. These
+  // branches used to decline silently (Question never surfaces, the caller just
+  // sees a decline), which made routing failures unobservable; log the reason
+  // and a routing snapshot so a decline here is diagnosable in production logs.
   if (!question || !sessionID) {
+    yield* Effect.logWarning("elicitation declined without surfacing", {
+      reason: !question ? "no Question service" : "no sessionID",
+      sessionContext: SessionContext.sessionID,
+      activeSessionFallback: activeSession.map((entry) => entry.id),
+    })
     return { action: "decline" } satisfies ElicitResponse
   }
 
@@ -263,6 +285,7 @@ export const handleElicitation = Effect.fn("MCP.elicitation.handle")(function* (
       )
     yield* SettingsHook.landSystemMessages(hookResult as TriggerResult, { sessionID })
     if ((hookResult as TriggerResult).blocked) {
+      yield* Effect.logWarning("elicitation declined: hook blocked")
       return { action: "decline" } satisfies ElicitResponse
     }
   }
@@ -319,7 +342,7 @@ export function registerElicitationHandler(
     params?: { message?: string; requestedSchema?: unknown; mode?: string }
   }) => {
     const params = request.params ?? {}
-    const sessionID = SessionContext.sessionID ?? activeSession
+    const sessionID = SessionContext.sessionID ?? activeSession.at(-1)?.id
     const response = await bridge.promise(
       handleElicitation({
         message: params.message ?? "",

packages/opencode/src/session/tools.ts

@@ -161,13 +161,10 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: {
               // effectiveArgs reflects any PreToolUse updatedInput rewrite (shallow merge).
               args = decision.effectiveArgs
             }
-            // Set the active session for server-initiated MCP elicitation. The
-            // SDK transport dispatch breaks AsyncLocalStorage, so the elicitation
-            // handler reads this module-level slot (set here, cleared below) to
-            // route the surfaced Question to this session.
-            setActiveElicitationSession(ctx.sessionID)
-            const result = yield* item.execute(args, ctx)
-            setActiveElicitationSession(undefined)
+            const result = yield* Effect.suspend(() => {
+              const cleanup = setActiveElicitationSession(ctx.sessionID)
+              return item.execute(args, ctx).pipe(Effect.ensuring(Effect.sync(cleanup)))
+            })
             const output = {
               ...result,
               attachments: result.attachments?.map((attachment) => ({
@@ -556,7 +553,10 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: {
           }
           const result: Awaited<ReturnType<NonNullable<typeof execute>>> = yield* Effect.gen(function* () {
             yield* ctx.ask({ permission: key, metadata: {}, patterns: ["*"], always: ["*"] })
-            return yield* Effect.promise(() => execute(args, opts))
+            return yield* Effect.suspend(() => {
+              const cleanup = setActiveElicitationSession(ctx.sessionID)
+              return Effect.promise(() => execute(args, opts)).pipe(Effect.ensuring(Effect.sync(cleanup)))
+            })
           }).pipe(
             Effect.withSpan("Tool.execute", {
               attributes: {

packages/opencode/test/mcp/elicitation-transport.test.ts

@@ -1,5 +1,5 @@
-import { afterEach, describe, expect } from "bun:test"
-import { Effect, Fiber, Layer, Exit } from "effect"
+import { afterEach, beforeEach, describe, expect } from "bun:test"
+import { Cause, Effect, Fiber, Layer, Exit } from "effect"
 import { Client } from "@modelcontextprotocol/sdk/client/index.js"
 import { Server } from "@modelcontextprotocol/sdk/server/index.js"
 import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js"
@@ -8,7 +8,6 @@ import { Question } from "@/question"
 import { Notification } from "@/notification"
 import { SettingsHook, type HookPayload } from "@/hook/settings"
 import { registerElicitationHandler, setActiveElicitationSession } from "@/mcp/elicitation"
-import { SessionContext } from "@/effect/session-context"
 import { EventV2Bridge } from "@/event-v2-bridge"
 import { EffectBridge } from "@/effect/bridge"
 import { disposeAllInstances, testInstanceStoreLayer } from "../fixture/fixture"
@@ -30,7 +29,16 @@ import { pollWithTimeout, testEffect } from "../lib/effect"
 // Question.ask during a tool call is exactly the "during a streaming turn" shape —
 // an MCP tool blocks on elicitation exactly as it blocks on any slow operation.
 
+beforeEach(() => {
+  // Explicit pre-test reset. bun runs the whole suite in one process, so a
+  // module-level activeSession slot left behind by a prior file would route
+  // this test's elicitation to the wrong session (or none) before callTool
+  // gets to set the fallback. afterEach alone only cleans up after each test.
+  setActiveElicitationSession(undefined)
+})
+
 afterEach(async () => {
+  setActiveElicitationSession(undefined)
   await disposeAllInstances()
 })
 
@@ -64,6 +72,7 @@ const env = Layer.mergeAll(
 const it = testEffect(env)
 
 const SESSION = "ses_elicitation_transport"
+const STALE_SESSION = "ses_elicitation_transport_stale"
 
 /**
  * Build a stub MCP server whose only tool, "pick", elicits a color choice from
@@ -98,6 +107,17 @@ const awaitValue = <A, E>(fiber: Fiber.Fiber<A, E>) =>
     return exit.value
   })
 
+// Non-blocking snapshot of a fiber's state for timeout diagnostics: resolved
+// (with its result text), failed (with the cause), or still pending. Used to
+// tell a silent decline (callTool resolved with `elicit decline`) apart from a
+// genuine hang (still in flight) when the surfacing poll times out.
+const describeFiber = <A, E>(fiber: Fiber.Fiber<A, E>): string => {
+  const exit = fiber.pollUnsafe()
+  if (exit === undefined) return "pending (tool call still in flight)"
+  if (Exit.isSuccess(exit)) return `resolved: ${JSON.stringify((exit.value as { content?: unknown })?.content)}`
+  return `failed: ${Cause.pretty(exit.cause)}`
+}
+
 describe("mcp elicitation — real transport round-trip (5.4)", () => {
   it.instance("server elicits mid-tool-call; client surfaces, user replies, accept round-trips", () =>
     Effect.gen(function* () {
@@ -114,28 +134,54 @@ describe("mcp elicitation — real transport round-trip (5.4)", () => {
       )
       registerElicitationHandler(client, bridge)
       const server = makeStubServer()
+      yield* Effect.addFinalizer(() =>
+        Effect.promise(() => Promise.all([client.close(), server.close()])).pipe(Effect.catch(() => Effect.void)),
+      )
       const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair()
       yield* Effect.promise(() => Promise.all([client.connect(clientTransport), server.connect(serverTransport)]))
 
       // Drive the tool call. Production MCP tool execution sets the active
       // session synchronously around the server call (the SDK transport dispatch
       // breaks AsyncLocalStorage, so SessionContext alone is insufficient — see
-      // elicitation.ts). The test mirrors that by setting the slot directly.
+      // elicitation.ts). The test mirrors that by setting the fallback directly.
       const before = rec.recorded.length
-      setActiveElicitationSession(SESSION)
-      const callFiber = yield* Effect.promise(() =>
-        SessionContext.run(SESSION, () => client.callTool({ name: "pick", arguments: {} })),
-      ).pipe(Effect.forkScoped)
+      const cleanup = setActiveElicitationSession(SESSION)
+      yield* Effect.addFinalizer(() => Effect.sync(cleanup))
+      const staleCleanup = setActiveElicitationSession(STALE_SESSION)
+      yield* Effect.addFinalizer(() => Effect.sync(staleCleanup))
+      staleCleanup()
+      const callFiber = yield* Effect.promise(() => client.callTool({ name: "pick", arguments: {} })).pipe(Effect.forkScoped)
 
-      // The elicitation surfaces as a pending Question.
+      // The elicitation surfaces as a pending Question. Filter by this test's
+      // SESSION so a pending question leaked from a prior file (bun runs the
+      // full suite in one process) can't trip the count check; the total list
+      // is retained for the timeout diagnostic below.
+      let lastItems: readonly Question.Request[] = []
       const pending = yield* pollWithTimeout(
         Effect.gen(function* () {
-          const items = yield* question.list()
-          return items.length === 1 ? (items as readonly Question.Request[]) : undefined
+          const items = (yield* question.list()) as readonly Question.Request[]
+          lastItems = items
+          const mine = items.filter((x) => String(x.sessionID) === SESSION)
+          return mine.length === 1 ? mine : undefined
         }),
         "elicitation never surfaced as a Question",
         "15 seconds",
+      ).pipe(
+        // Self-diagnose on timeout: the callTool fiber's non-blocking poll
+        // separates a silent decline (resolved with `elicit decline`) from a
+        // genuine hang (still pending), and the list snapshot shows whether
+        // the Question surfaced at all.
+        Effect.catch(
+          () =>
+            Effect.fail(
+              new Error(
+                "elicitation never surfaced as a Question — " +
+                  `question.list()=${JSON.stringify(lastItems)}; callFiber: ${describeFiber(callFiber)}`,
+              ),
+            ),
+        ),
       )
+      expect(String(pending[0].sessionID)).toBe(SESSION)
 
       // Reply "green" → adapter validates, accepts, returns content to the server.
       yield* question.reply({ requestID: pending[0].id, answers: [["green"]] })
@@ -154,9 +200,6 @@ describe("mcp elicitation — real transport round-trip (5.4)", () => {
           (p) => p.event === "ElicitationResult" && JSON.stringify((p as { result?: unknown }).result).includes("green"),
         ),
       ).toHaveLength(1)
-
-      // Cleanup the connected pair.
-      yield* Effect.promise(() => Promise.all([client.close(), server.close()])).pipe(Effect.catch(() => Effect.void))
     }),
   )
 })