Skip to content

Commit fff0e55

Browse files
floodsungFlood Sungclaude
authored
fix(bridge): surface between-turn AskUserQuestion as its own card (#274)
When AskUserQuestion fires between user turns (teammate / /goal / continuation-burst follow-up), the PreToolUse hook in PersistentClaudeExecutor has no `activeTurn` to piggy-back on. The question text only landed inside the coalesced "Agent activity" body, and the user's typed reply was treated as a fresh user turn — which then blocked for 6 minutes on the still-hanging hook ("Thinking…" forever). Fix: when the hook trips with no activeTurn, additionally emit a `between-turn-question` event with the parsed questions payload. The bridge subscribes alongside the existing `spontaneous` / `continuation-turn` channels and: - sends a dedicated question card via sendQuestionCard - intercepts the user's next typed reply for that chatId in handleMessage (before runningTasks / queue branches) - routes the parsed answer back via executor.resolveQuestion(), unblocking the hook and letting the SDK proceed - tears the card down to error state if the executor is evicted Multi-sub-question between-turn calls only surface the first question (rare; logged) — same single-question-at-a-time pattern as runOneTurn. Co-authored-by: Flood Sung <floodsung@xvirobotics.ai> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
1 parent a6ce914 commit fff0e55

3 files changed

Lines changed: 524 additions & 0 deletions

File tree

src/bridge/message-bridge.ts

Lines changed: 225 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,25 @@ export class MessageBridge {
236236
cardMessageId: string;
237237
turnId: string;
238238
}>();
239+
/**
240+
* AskUserQuestion calls that fired between turns (no activeTurn in the
241+
* executor at the time of the PreToolUse hook). The bridge displays them
242+
* as standalone question cards and routes the user's next typed reply
243+
* back to {@link PersistentClaudeExecutor.resolveQuestion}.
244+
*
245+
* Without this, the question text would only appear inside the coalesced
246+
* "Agent activity" body and the user's reply would be treated as a fresh
247+
* user turn — which then blocks for 6 minutes on the still-hanging hook.
248+
*
249+
* Single in-flight slot per chatId. If a second between-turn question
250+
* fires while one is still pending, the later one wins — the older
251+
* resolver hangs until its 6-minute timeout (rare in practice).
252+
*/
253+
private pendingBetweenTurnQuestions = new Map<string, {
254+
toolUseId: string;
255+
questions: PendingQuestion['questions'];
256+
cardMessageId: string;
257+
}>();
239258
/** Callback for activity lifecycle events (task started/completed/failed). */
240259
onActivityEvent?: (event: ActivityEventData) => void;
241260

@@ -473,6 +492,19 @@ export class MessageBridge {
473492
if (cont) {
474493
cont.abortController.abort();
475494
}
495+
// Between-turn question whose resolver is now dead — flush the
496+
// question card to an error state and drop the bookkeeping so the
497+
// user's next message isn't intercepted as the answer.
498+
const q = this.pendingBetweenTurnQuestions.get(chatId);
499+
if (q) {
500+
this.pendingBetweenTurnQuestions.delete(chatId);
501+
void this.finalizeBetweenTurnQuestionCard(q.cardMessageId, {
502+
status: 'error',
503+
userPrompt: 'Question',
504+
responseText: '_Question canceled — agent session ended._',
505+
toolCalls: [],
506+
});
507+
}
476508
});
477509
this.logger.info(
478510
{
@@ -512,9 +544,190 @@ export class MessageBridge {
512544
// (card + stream loop + finalize). Errors are logged inside.
513545
void this.handleContinuationTurn(chatId, handle as ExecutionHandle);
514546
});
547+
exec.on('between-turn-question', (payload: {
548+
toolUseId: string;
549+
questions: PendingQuestion['questions'];
550+
}) => {
551+
void this.handleBetweenTurnQuestion(chatId, payload);
552+
});
515553
this.logger.debug({ chatId }, 'MessageBridge: attached executor subscriptions');
516554
}
517555

556+
/**
557+
* Surface a between-turn AskUserQuestion as its own card on the chat.
558+
* Called from the `between-turn-question` executor event. The user's
559+
* next typed reply for this chatId is intercepted in
560+
* {@link handleMessage} and routed back via the executor's
561+
* {@link PersistentClaudeExecutor.resolveQuestion}.
562+
*
563+
* Single in-flight slot per chatId — if one is already pending, the
564+
* older one is abandoned (its resolver will hang to the SDK's 6-min
565+
* timeout, then return empty answers). The older card is marked
566+
* "superseded" so the user sees what happened.
567+
*/
568+
private async handleBetweenTurnQuestion(
569+
chatId: string,
570+
payload: { toolUseId: string; questions: PendingQuestion['questions'] },
571+
): Promise<void> {
572+
if (!payload.questions || payload.questions.length === 0) {
573+
this.logger.warn({ chatId, toolUseId: payload.toolUseId }, 'between-turn question with no parsed questions; skipping card');
574+
return;
575+
}
576+
const existing = this.pendingBetweenTurnQuestions.get(chatId);
577+
if (existing) {
578+
this.logger.warn(
579+
{ chatId, prevToolUseId: existing.toolUseId, newToolUseId: payload.toolUseId },
580+
'MessageBridge: between-turn question superseded by newer one',
581+
);
582+
void this.finalizeBetweenTurnQuestionCard(existing.cardMessageId, {
583+
status: 'error',
584+
userPrompt: 'Question',
585+
responseText: '_Superseded by a newer question._',
586+
toolCalls: [],
587+
});
588+
this.pendingBetweenTurnQuestions.delete(chatId);
589+
}
590+
591+
// Show only the first question on the card (matches runOneTurn — the
592+
// bridge surfaces one sub-question at a time, advancing the card on
593+
// each typed reply). Multi-question case is logged below; the existing
594+
// bridge code path doesn't support advancing between-turn sub-questions
595+
// yet, so we route only the first answer and short-circuit the rest.
596+
if (payload.questions.length > 1) {
597+
this.logger.warn(
598+
{ chatId, toolUseId: payload.toolUseId, total: payload.questions.length },
599+
'between-turn AskUserQuestion has multiple sub-questions; only the first will be displayed and routed',
600+
);
601+
}
602+
const displayQuestion: PendingQuestion = {
603+
toolUseId: payload.toolUseId,
604+
questions: [payload.questions[0]],
605+
};
606+
607+
const card: CardState = {
608+
status: 'waiting_for_input',
609+
userPrompt: '(between-turn question)',
610+
responseText: '',
611+
toolCalls: [],
612+
pendingQuestion: displayQuestion,
613+
};
614+
615+
const send = this.sender.sendQuestionCard
616+
? this.sender.sendQuestionCard.bind(this.sender)
617+
: this.sender.sendCard.bind(this.sender);
618+
let cardMessageId: string | undefined;
619+
try {
620+
cardMessageId = await send(chatId, card);
621+
} catch (err) {
622+
this.logger.error({ err, chatId, toolUseId: payload.toolUseId }, 'MessageBridge: failed to send between-turn question card');
623+
return;
624+
}
625+
if (!cardMessageId) {
626+
this.logger.warn({ chatId, toolUseId: payload.toolUseId }, 'MessageBridge: between-turn question card returned no messageId');
627+
return;
628+
}
629+
630+
this.pendingBetweenTurnQuestions.set(chatId, {
631+
toolUseId: payload.toolUseId,
632+
questions: payload.questions,
633+
cardMessageId,
634+
});
635+
this.logger.info(
636+
{ chatId, toolUseId: payload.toolUseId, cardMessageId },
637+
'MessageBridge: between-turn question card opened',
638+
);
639+
}
640+
641+
/**
642+
* Update the dedicated question card after the user answers (or after the
643+
* executor is torn down). Uses updateQuestionCard if the sender supports
644+
* it, else falls back to updateCard.
645+
*/
646+
private async finalizeBetweenTurnQuestionCard(
647+
cardMessageId: string,
648+
state: CardState,
649+
): Promise<void> {
650+
try {
651+
const update = this.sender.updateQuestionCard
652+
? this.sender.updateQuestionCard.bind(this.sender)
653+
: this.sender.updateCard.bind(this.sender);
654+
await update(cardMessageId, state);
655+
} catch (err) {
656+
this.logger.warn({ err, cardMessageId }, 'MessageBridge: failed to update between-turn question card');
657+
}
658+
}
659+
660+
/**
661+
* Treat the user's typed reply as the answer to a pending between-turn
662+
* question. Routes through {@link PersistentClaudeExecutor.resolveQuestion}
663+
* so the AskUserQuestion PreToolUse hook unblocks and the SDK proceeds.
664+
* Returns true if the message was consumed as an answer (caller should
665+
* NOT continue to executeQuery).
666+
*/
667+
private async tryHandleBetweenTurnQuestionReply(msg: IncomingMessage): Promise<boolean> {
668+
const { chatId, text, imageKey } = msg;
669+
const pending = this.pendingBetweenTurnQuestions.get(chatId);
670+
if (!pending) return false;
671+
672+
// Image-only reply isn't a valid answer; nudge the user.
673+
if (imageKey && !text.trim()) {
674+
await this.sender.sendText(chatId, '请用文字回复问题卡片中的选项编号或自定义答案。');
675+
return true;
676+
}
677+
678+
const trimmed = text.trim();
679+
const firstQ = pending.questions[0];
680+
let answerText: string;
681+
const num = parseInt(trimmed, 10);
682+
if (Number.isFinite(num) && num >= 1 && num <= firstQ.options.length) {
683+
answerText = firstQ.options[num - 1].label;
684+
} else {
685+
answerText = trimmed;
686+
}
687+
688+
const answers: Record<string, string> = { [firstQ.header]: answerText };
689+
// For multi-sub-question between-turn calls (rare; logged on arrival),
690+
// synthesize empty answers for the rest so the hook still resolves.
691+
for (let i = 1; i < pending.questions.length; i++) {
692+
answers[pending.questions[i].header] = '';
693+
}
694+
695+
const executor = this.persistentRegistry?.peek(chatId);
696+
if (!executor) {
697+
this.logger.warn(
698+
{ chatId, toolUseId: pending.toolUseId },
699+
'MessageBridge: between-turn answer arrived but executor is gone; dropping',
700+
);
701+
this.pendingBetweenTurnQuestions.delete(chatId);
702+
await this.finalizeBetweenTurnQuestionCard(pending.cardMessageId, {
703+
status: 'error',
704+
userPrompt: 'Question',
705+
responseText: '_Question canceled — agent session ended._',
706+
toolCalls: [],
707+
});
708+
return true;
709+
}
710+
711+
this.pendingBetweenTurnQuestions.delete(chatId);
712+
try {
713+
executor.resolveQuestion(pending.toolUseId, answers);
714+
} catch (err) {
715+
this.logger.error({ err, chatId, toolUseId: pending.toolUseId }, 'MessageBridge: resolveQuestion threw');
716+
}
717+
this.logger.info(
718+
{ chatId, toolUseId: pending.toolUseId, answer: answerText },
719+
'MessageBridge: resolved between-turn question',
720+
);
721+
722+
await this.finalizeBetweenTurnQuestionCard(pending.cardMessageId, {
723+
status: 'complete',
724+
userPrompt: 'Question',
725+
responseText: `> **Reply:** ${answerText}`,
726+
toolCalls: [],
727+
});
728+
return true;
729+
}
730+
518731
/**
519732
* Buffer a spontaneous message and (re)arm the coalesce timer. We extract
520733
* just the human-readable bits — assistant text and tool_use intent —
@@ -1011,6 +1224,18 @@ export class MessageBridge {
10111224
return;
10121225
}
10131226

1227+
// Between-turn AskUserQuestion reply — must run BEFORE the
1228+
// running-task / queue branches below, because no `runningTasks` entry
1229+
// exists for between-turn questions (they fire from the persistent
1230+
// executor outside of any user-initiated turn). If we let the message
1231+
// fall through, it would spawn a fresh turn that immediately blocks on
1232+
// the still-hanging hook for 6 minutes. See:
1233+
// [[bug_feishu_v2_mobile_action_buttons]] history and
1234+
// PersistentClaudeExecutor.askUserQuestionHook.
1235+
if (await this.tryHandleBetweenTurnQuestionReply(msg)) {
1236+
return;
1237+
}
1238+
10141239
// Check if there's a pending question waiting for an answer
10151240
const task = this.runningTasks.get(chatId);
10161241
if (task && task.pendingQuestion) {

src/engines/claude/persistent-executor.ts

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,47 @@ export function classifyBurstSource(raw: unknown): BurstSource {
255255
return 'spontaneous';
256256
}
257257

258+
/**
259+
* Parse the `tool_input` payload of an AskUserQuestion PreToolUse hook into
260+
* the bridge's PendingQuestion['questions'] shape. Mirrors stream-processor's
261+
* `extractPendingQuestion` (kept separate so the persistent executor doesn't
262+
* have to import from stream-processor).
263+
*
264+
* Exported for unit tests; not part of the public executor API.
265+
*/
266+
export function parseAskUserQuestionInput(input: unknown): Array<{
267+
question: string;
268+
header: string;
269+
options: Array<{ label: string; description: string }>;
270+
multiSelect: boolean;
271+
}> {
272+
if (!input || typeof input !== 'object') return [];
273+
const inp = input as Record<string, unknown>;
274+
const questions = inp.questions;
275+
if (!Array.isArray(questions)) return [];
276+
return questions.map((q: any) => ({
277+
question: String(q?.question || ''),
278+
header: String(q?.header || ''),
279+
options: Array.isArray(q?.options)
280+
? q.options.map((o: any) => ({
281+
label: String(o?.label || ''),
282+
description: String(o?.description || ''),
283+
}))
284+
: [],
285+
multiSelect: Boolean(q?.multiSelect),
286+
}));
287+
}
288+
289+
/**
290+
* Payload of the `between-turn-question` event. Structurally identical to
291+
* {@link PendingQuestion} (src/types.ts); a separate name avoids a coupling
292+
* import from the bridge types.
293+
*/
294+
export interface BetweenTurnQuestionEvent {
295+
toolUseId: string;
296+
questions: ReturnType<typeof parseAskUserQuestionInput>;
297+
}
298+
258299
export class PersistentClaudeExecutor extends EventEmitter {
259300
private state: ExecutorState = 'starting';
260301
private inputQueue: AsyncQueue<SDKUserMessage>;
@@ -660,6 +701,18 @@ export class PersistentClaudeExecutor extends EventEmitter {
660701
// marks AskUserQuestion as requiresUserInteraction; in bypassPermissions
661702
// mode we intercept, pause until the bridge supplies the user's answers,
662703
// then return them as updatedInput so the SDK auto-allows.
704+
//
705+
// Between-turn fire: if the hook trips while no activeTurn is in flight
706+
// (teammate / `/goal` / continuation-burst follow-up), we additionally
707+
// emit `between-turn-question` so the bridge can mount a dedicated
708+
// question card on the chat. Without this side-channel the question text
709+
// only lands in the coalesced "Agent activity" card body and the user's
710+
// typed reply gets treated as a fresh user turn (which then blocks for
711+
// 6 minutes on this still-hanging hook). The resolver registration path
712+
// below is unchanged — the bridge calls resolveQuestion() to feed answers
713+
// back through the same `updatedInput` mechanism. The emitted event
714+
// shape matches PendingQuestion (src/types.ts) so the bridge can use it
715+
// verbatim in CardState.
663716
const askUserQuestionHook = async (
664717
input: { hook_event_name: string; tool_name: string; tool_input: unknown; tool_use_id: string },
665718
_toolUseId: string | undefined,
@@ -669,6 +722,15 @@ export class PersistentClaudeExecutor extends EventEmitter {
669722
const id = input.tool_use_id;
670723
const answers = await new Promise<Record<string, string>>((resolve) => {
671724
this.pendingQuestionResolvers.set(id, resolve);
725+
// Surface the question to the bridge if we're between turns (no
726+
// listener owns the live stream — the agent_activity coalesce card
727+
// would otherwise eat the question silently). Done AFTER setting
728+
// the resolver so a fast bridge reply can't race past it.
729+
if (!this.activeTurn) {
730+
const parsed = parseAskUserQuestionInput(toolInput);
731+
log.info({ toolUseId: id, questionCount: parsed.length }, 'PersistentExecutor: between-turn AskUserQuestion');
732+
this.emit('between-turn-question', { toolUseId: id, questions: parsed });
733+
}
672734
const timeout = setTimeout(() => {
673735
if (this.pendingQuestionResolvers.delete(id)) {
674736
log.warn({ toolUseId: id }, 'AskUserQuestion hook timed out (6 min) — empty answers');

0 commit comments

Comments
 (0)