Skip to content

Commit 903c510

Browse files
saravmajesticclaude
andcommitted
fix: [AI-7743] keep Bus mirror off the question Deferred critical path
Address review feedback (flagged by all reviewers): the `Bus.publish` mirrors added for /event (webview) clients sat on the Deferred critical path. Since `Effect.promise` converts a promise rejection into an unrecoverable fiber defect, a `Bus.publish` rejection could abort the fiber before `Deferred.succeed`/`Deferred.fail` ran — leaving the awaiting `Question.ask()` hung on "Thinking…", the exact failure this PR fixes. - Add a best-effort `mirror()` helper: `Effect.promise(() => Bus.publish(...))` recovered with `Effect.catchCause` to a logged warning, so a publish failure can never abort core question settlement. - Settle the Deferred FIRST, then mirror, in `reply()` and `reject()`. - `ask()` uses `mirror()` too, so a publish failure can't abort it before the cleanup finalizer is registered. - Replace `z.any()` with `z.unknown()` in the `question.asked` Bus schema. - Log (not silently swallow) failures in the instance-dispose cleanup. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent cd96fea commit 903c510

1 file changed

Lines changed: 38 additions & 20 deletions

File tree

packages/opencode/src/question/index.ts

Lines changed: 38 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ const BusAsked = BusEvent.define(
112112
z.object({
113113
id: QuestionID.zod,
114114
sessionID: SessionID.zod,
115-
questions: z.array(z.any()),
115+
questions: z.array(z.unknown()),
116116
tool: z.object({ messageID: MessageID.zod, callID: z.string() }).optional(),
117117
}),
118118
)
@@ -128,6 +128,17 @@ const BusRejected = BusEvent.define(
128128
"question.rejected",
129129
z.object({ sessionID: SessionID.zod, requestID: QuestionID.zod }),
130130
)
131+
132+
// Best-effort Bus mirror. A fire-and-forget /event notification must NEVER be
133+
// able to abort core question settlement: `Effect.promise` turns a promise
134+
// rejection into an unrecoverable fiber defect, and on the Deferred critical
135+
// path that would skip `Deferred.succeed`/`Deferred.fail` and re-hang the tool
136+
// on "Thinking…" — the exact failure this PR fixes. Recover any cause to a
137+
// logged warning so publication can never block settlement.
138+
const mirror = <D extends BusEvent.Definition>(def: D, properties: z.output<D["properties"]>) =>
139+
Effect.promise(() => Bus.publish(def, properties)).pipe(
140+
Effect.catchCause((cause) => Effect.logWarning("question bus mirror failed", { type: def.type, cause })),
141+
)
131142
// altimate_change end
132143

133144
export class RejectedError extends Schema.TaggedErrorClass<RejectedError>()("QuestionRejectedError", {}) {
@@ -202,7 +213,11 @@ export const layer = Layer.effect(
202213
if (!map) return
203214
pendingByDir.delete(directory)
204215
for (const { deferred } of map.values()) {
205-
await Effect.runPromise(Deferred.fail(deferred, new RejectedError())).catch(() => {})
216+
await Effect.runPromise(
217+
Deferred.fail(deferred, new RejectedError()).pipe(
218+
Effect.catchCause((cause) => Effect.logWarning("question cleanup failed on dispose", { cause })),
219+
),
220+
)
206221
}
207222
})
208223
yield* Effect.addFinalizer(() => Effect.sync(off))
@@ -229,11 +244,16 @@ export const layer = Layer.effect(
229244
}
230245
pending.set(id, { info, deferred })
231246
yield* events.publish(Event.Asked, info)
232-
// altimate_change start — also publish on the Bus wildcard so the IDE webview
247+
// altimate_change start — also mirror on the Bus wildcard so the IDE webview
233248
// (subscribed to /event) receives question.asked and can answer the card.
234-
yield* Effect.promise(() =>
235-
Bus.publish(BusAsked, { id, sessionID: input.sessionID, questions: [...input.questions], tool: input.tool }),
236-
)
249+
// Best-effort: a publish failure must not abort ask() before it registers
250+
// the cleanup finalizer below (see `mirror`).
251+
yield* mirror(BusAsked, {
252+
id,
253+
sessionID: input.sessionID,
254+
questions: [...input.questions],
255+
tool: input.tool,
256+
})
237257
// altimate_change end
238258

239259
return yield* Effect.ensuring(
@@ -263,16 +283,15 @@ export const layer = Layer.effect(
263283
requestID: existing.info.id,
264284
answers: input.answers.map((a) => [...a]),
265285
})
266-
// altimate_change start — mirror on the Bus wildcard for /event (webview) clients.
267-
yield* Effect.promise(() =>
268-
Bus.publish(BusReplied, {
269-
sessionID: existing.info.sessionID,
270-
requestID: existing.info.id,
271-
answers: input.answers.map((a) => [...a]),
272-
}),
273-
)
274-
// altimate_change end
275286
yield* Deferred.succeed(existing.deferred, input.answers)
287+
// altimate_change start — mirror on the Bus wildcard for /event (webview) clients,
288+
// AFTER settling the Deferred and best-effort so a publish failure can't re-hang ask().
289+
yield* mirror(BusReplied, {
290+
sessionID: existing.info.sessionID,
291+
requestID: existing.info.id,
292+
answers: input.answers.map((a) => [...a]),
293+
})
294+
// altimate_change end
276295
})
277296

278297
const reject = Effect.fn("Question.reject")(function* (requestID: QuestionID) {
@@ -290,12 +309,11 @@ export const layer = Layer.effect(
290309
sessionID: existing.info.sessionID,
291310
requestID: existing.info.id,
292311
})
293-
// altimate_change start — mirror on the Bus wildcard for /event (webview) clients.
294-
yield* Effect.promise(() =>
295-
Bus.publish(BusRejected, { sessionID: existing.info.sessionID, requestID: existing.info.id }),
296-
)
297-
// altimate_change end
298312
yield* Deferred.fail(existing.deferred, new RejectedError())
313+
// altimate_change start — mirror on the Bus wildcard for /event (webview) clients,
314+
// AFTER settling the Deferred and best-effort so a publish failure can't strand it.
315+
yield* mirror(BusRejected, { sessionID: existing.info.sessionID, requestID: existing.info.id })
316+
// altimate_change end
299317
})
300318

301319
const list = Effect.fn("Question.list")(function* () {

0 commit comments

Comments
 (0)