Skip to content

Commit ab2bd9f

Browse files
saravmajesticclaude
andcommitted
fix: [AI-7743] mcp-add question hangs — share question state + event bus across bundled module copies
`/discover-and-add-mcps` rendered the "which MCP / what scope" question but, after answering, the chat sat on "Thinking…" and the server was never added. Root cause: after the v1.17.9 upstream merge, `src/question/index.ts` is bundled as TWO separate module instances (a module-scoped id differs between the tool's `ask()` and the HTTP route's `reply()`; normalizing to `@/question` imports did NOT make Bun dedupe it). Each copy ran its own `makeRuntime` runtime, which broke the flow two independent ways: 1. State split — the `question` tool registered the pending `Deferred` in one copy's `InstanceState` map, while `POST /question/:id/reply` looked it up in the OTHER copy's empty map. The reply 404'd and the `Deferred` never resolved, so the agent loop blocked forever ("Thinking…"). 2. Event never reached the IDE webview — `question.asked` was published only via `EventV2Bridge` -> `GlobalBus`, but the `/event` SSE stream the webview reads is fed by the Bus *wildcard PubSub* (`Bus.publish`). So `pendingQuestions` stayed empty and the answer card's Submit had no request id -> submitting did nothing. Fix (all in `question/index.ts`, plus import cleanup): - Anchor the pending-question registry on `globalThis` (keyed by instance directory) so every module copy shares one map. Restore per-instance cleanup via `registerDisposer` so entries don't leak across instances/tests. - Publish `question.asked`/`replied`/`rejected` via `Bus.publish` (added `BusEvent` mirrors) so they reach `/event` like every other webview-visible event. - Normalize the remaining relative `../question` imports to `@/question`. Verified end-to-end in code-server: discover -> answer (Yes / Project) -> datamate written to `.altimate-code/altimate-code.json` (enabled) and the chat shows the success summary. 55 question unit tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 0dc8850 commit ab2bd9f

4 files changed

Lines changed: 100 additions & 27 deletions

File tree

packages/opencode/src/question/index.ts

Lines changed: 97 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ import { EventV2Bridge } from "@/event-v2-bridge"
77
import { EventV2 } from "@opencode-ai/core/event"
88
// altimate_change start — makeRuntime for the restored Promise wrappers (see bottom of file)
99
import { makeRuntime } from "@/effect/run-service"
10+
import { registerDisposer } from "@/effect/instance-registry"
11+
import { Bus } from "@/bus"
12+
import { BusEvent } from "@/bus/bus-event"
13+
import z from "zod"
1014
// altimate_change end
1115

1216
// Schemas — these are pure data; nothing checks class identity (see PR
@@ -93,6 +97,39 @@ export const Event = {
9397
Rejected: EventV2.define({ type: "question.rejected", schema: Rejected.fields }),
9498
}
9599

100+
// altimate_change start — BusEvent mirrors of the question events.
101+
//
102+
// The EventV2 `Event` defs above publish to GlobalBus/EventV2 consumers only.
103+
// The IDE webview subscribes to the `/event` SSE route, which is fed by the Bus
104+
// *wildcard PubSub* (`Bus.publish`), a different channel. So `question.asked`
105+
// never reached the webview → `pendingQuestions` stayed empty → the mcp-add
106+
// question card had no request id to reply with → "submit does nothing".
107+
// Publish these via `Bus.publish` too so they reach /event like every other
108+
// webview-visible event. (Schemas are loose — Bus.publish does not re-validate;
109+
// they exist for the `type` string and typing.)
110+
const BusAsked = BusEvent.define(
111+
"question.asked",
112+
z.object({
113+
id: QuestionID.zod,
114+
sessionID: SessionID.zod,
115+
questions: z.array(z.any()),
116+
tool: z.object({ messageID: MessageID.zod, callID: z.string() }).optional(),
117+
}),
118+
)
119+
const BusReplied = BusEvent.define(
120+
"question.replied",
121+
z.object({
122+
sessionID: SessionID.zod,
123+
requestID: QuestionID.zod,
124+
answers: z.array(z.array(z.string())),
125+
}),
126+
)
127+
const BusRejected = BusEvent.define(
128+
"question.rejected",
129+
z.object({ sessionID: SessionID.zod, requestID: QuestionID.zod }),
130+
)
131+
// altimate_change end
132+
96133
export class RejectedError extends Schema.TaggedErrorClass<RejectedError>()("QuestionRejectedError", {}) {
97134
override get message() {
98135
return "The user dismissed this question"
@@ -108,9 +145,31 @@ interface PendingEntry {
108145
deferred: Deferred.Deferred<ReadonlyArray<Answer>, RejectedError>
109146
}
110147

111-
interface State {
112-
pending: Map<QuestionID, PendingEntry>
148+
// altimate_change start — process-global pending registry.
149+
//
150+
// The imperative `Question.ask()`/`reply()`/`list()` wrappers (bottom of file)
151+
// get bundled into MORE THAN ONE module instance (proven: a module-scoped id
152+
// differs between the tool's ask() and the HTTP route's reply()). Consistent
153+
// `@/question` imports did NOT dedupe them. Each copy ran its own module-scoped
154+
// `makeRuntime(...)` runtime with its own `InstanceState` cache, so the question
155+
// TOOL registered the pending Deferred in one copy's map while the HTTP reply
156+
// route looked it up in the OTHER copy's empty map — the Deferred never resolved
157+
// and the `/discover-and-add-mcps` question hung on "Thinking…" after answering.
158+
//
159+
// Anchor the registry on `globalThis` so every module copy shares one Map. Keyed
160+
// by instance directory so `list()` stays per-instance.
161+
type PendingByDir = Map<string, Map<QuestionID, PendingEntry>>
162+
const pendingByDir: PendingByDir = ((globalThis as Record<string, unknown>)["__altimateQuestionPending"] ??=
163+
new Map<string, Map<QuestionID, PendingEntry>>()) as PendingByDir
164+
function pendingFor(directory: string): Map<QuestionID, PendingEntry> {
165+
let map = pendingByDir.get(directory)
166+
if (!map) {
167+
map = new Map<QuestionID, PendingEntry>()
168+
pendingByDir.set(directory, map)
169+
}
170+
return map
113171
}
172+
// altimate_change end
114173

115174
// Service
116175

@@ -134,31 +193,28 @@ export const layer = Layer.effect(
134193
Service,
135194
Effect.gen(function* () {
136195
const events = yield* EventV2Bridge.Service
137-
const state = yield* InstanceState.make<State>(
138-
Effect.fn("Question.state")(function* () {
139-
const state = {
140-
pending: new Map<QuestionID, PendingEntry>(),
141-
}
142-
143-
yield* Effect.addFinalizer(() =>
144-
Effect.gen(function* () {
145-
for (const item of state.pending.values()) {
146-
yield* Deferred.fail(item.deferred, new RejectedError())
147-
}
148-
state.pending.clear()
149-
}),
150-
)
151-
152-
return state
153-
}),
154-
)
196+
197+
// altimate_change start — clear a directory's pending questions when its
198+
// instance is disposed/reloaded (mirrors the removed InstanceState finalizer)
199+
// so entries in the process-global registry don't leak across instances.
200+
const off = registerDisposer(async (directory) => {
201+
const map = pendingByDir.get(directory)
202+
if (!map) return
203+
pendingByDir.delete(directory)
204+
for (const { deferred } of map.values()) {
205+
await Effect.runPromise(Deferred.fail(deferred, new RejectedError())).catch(() => {})
206+
}
207+
})
208+
yield* Effect.addFinalizer(() => Effect.sync(off))
209+
// altimate_change end
155210

156211
const ask = Effect.fn("Question.ask")(function* (input: {
157212
sessionID: SessionID
158213
questions: ReadonlyArray<Info>
159214
tool?: Tool
160215
}) {
161-
const pending = (yield* InstanceState.get(state)).pending
216+
const directory = yield* InstanceState.directory
217+
const pending = pendingFor(directory)
162218
const id = QuestionID.ascending()
163219
yield* Effect.logInfo("asking", { id, questions: input.questions.length })
164220

@@ -171,6 +227,11 @@ export const layer = Layer.effect(
171227
}
172228
pending.set(id, { info, deferred })
173229
yield* events.publish(Event.Asked, info)
230+
// altimate_change — also publish on the Bus wildcard so the IDE webview
231+
// (subscribed to /event) receives question.asked and can answer the card.
232+
yield* Effect.promise(() =>
233+
Bus.publish(BusAsked, { id, sessionID: input.sessionID, questions: [...input.questions], tool: input.tool }),
234+
)
174235

175236
return yield* Effect.ensuring(
176237
Deferred.await(deferred),
@@ -184,7 +245,7 @@ export const layer = Layer.effect(
184245
requestID: QuestionID
185246
answers: ReadonlyArray<Answer>
186247
}) {
187-
const pending = (yield* InstanceState.get(state)).pending
248+
const pending = pendingFor(yield* InstanceState.directory)
188249
const existing = pending.get(input.requestID)
189250
if (!existing) {
190251
yield* Effect.logWarning("reply for unknown request", { requestID: input.requestID })
@@ -197,11 +258,19 @@ export const layer = Layer.effect(
197258
requestID: existing.info.id,
198259
answers: input.answers.map((a) => [...a]),
199260
})
261+
// altimate_change — mirror on the Bus wildcard for /event (webview) clients.
262+
yield* Effect.promise(() =>
263+
Bus.publish(BusReplied, {
264+
sessionID: existing.info.sessionID,
265+
requestID: existing.info.id,
266+
answers: input.answers.map((a) => [...a]),
267+
}),
268+
)
200269
yield* Deferred.succeed(existing.deferred, input.answers)
201270
})
202271

203272
const reject = Effect.fn("Question.reject")(function* (requestID: QuestionID) {
204-
const pending = (yield* InstanceState.get(state)).pending
273+
const pending = pendingFor(yield* InstanceState.directory)
205274
const existing = pending.get(requestID)
206275
if (!existing) {
207276
yield* Effect.logWarning("reject for unknown request", { requestID })
@@ -213,11 +282,15 @@ export const layer = Layer.effect(
213282
sessionID: existing.info.sessionID,
214283
requestID: existing.info.id,
215284
})
285+
// altimate_change — mirror on the Bus wildcard for /event (webview) clients.
286+
yield* Effect.promise(() =>
287+
Bus.publish(BusRejected, { sessionID: existing.info.sessionID, requestID: existing.info.id }),
288+
)
216289
yield* Deferred.fail(existing.deferred, new RejectedError())
217290
})
218291

219292
const list = Effect.fn("Question.list")(function* () {
220-
const pending = (yield* InstanceState.get(state)).pending
293+
const pending = pendingFor(yield* InstanceState.directory)
221294
return Array.from(pending.values(), (x) => x.info)
222295
})
223296

packages/opencode/src/server/routes/question.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { Hono } from "hono"
22
import { describeRoute, validator } from "hono-openapi"
33
import { resolver } from "hono-openapi"
44
import { QuestionID } from "@/question/schema"
5-
import { Question } from "../../question"
5+
import { Question } from "@/question"
66
import z from "zod"
77
import { errors } from "../error"
88
import { lazy } from "../../util/lazy"

packages/opencode/src/tool/plan.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import z from "zod"
22
import path from "path"
33
import { Tool } from "./tool"
4-
import { Question } from "../question"
4+
import { Question } from "@/question"
55
import { Session } from "../session"
66
import { MessageV2 } from "../session/message-v2"
77
import { Provider } from "../provider/provider"

packages/opencode/src/tool/question.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import z from "zod"
22
import { Tool } from "./tool"
3-
import { Question } from "../question"
3+
import { Question } from "@/question"
44
import DESCRIPTION from "./question.txt"
55

66
// altimate_change start — zod mirror of Question.Prompt (Info minus `custom`).

0 commit comments

Comments
 (0)