fix(cli): serialize subagent confirmation focus to prevent concurrent input conflicts#2930
fix(cli): serialize subagent confirmation focus to prevent concurrent input conflicts#2930pic4xiu wants to merge 3 commits intoQwenLM:mainfrom
Conversation
| {!isFocused && ( | ||
| <Box marginBottom={0}> | ||
| <Text color={theme.text.secondary} dimColor> | ||
| ⏳ Waiting for other approval... | ||
| </Text> | ||
| </Box> | ||
| )} | ||
| <ToolConfirmationMessage |
There was a problem hiding this comment.
[Suggestion] isFocused is currently carrying two meanings: generic UI focus and ownership of the serialized approval lock. In AgentExecutionDisplay, any !isFocused renders ⏳ Waiting for other approval..., so the UI can claim another approval is blocking interaction even when this message is simply unfocused.
Consider passing a dedicated prop for the approval-lock state (for example isWaitingForOtherApproval or hasConfirmationFocusLock) and only showing the waiting message when another approval actually holds the lock.
wenshao
left a comment
There was a problem hiding this comment.
This PR fixes the parallel subagent confirmation focus bug by serializing focus through the relevant CLI UI components.
One review comment was left: the new waiting-state message currently conflates generic component focus with ownership of the serialized approval lock, which can make the UI claim it is waiting on another approval even when it is merely unfocused.
Overall verdict: Comment.
|
@wenshao Thanks for the feedback! I've addressed your suggestion in commit d7c92e2 by introducing a dedicated The UI now only shows the "⏳ Waiting for other approval..." indicator when another subagent actually holds the confirmation lock, rather than relying on Could you please take another look when you have a chance? Thanks! |
wenshao
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
Reviewed by gpt-5.4 via Qwen Code /review
…t conflicts When multiple subagents run in parallel and each triggers a confirmation prompt, all prompts previously received keyboard focus simultaneously, causing a single keypress to be dispatched to every active confirmation. This change introduces a first-come-first-served focus lock mechanism: - Track subagents with pending confirmations via a type guard - Use a useRef-based lock so only one confirmation is focused at a time - Automatically promote focus to the next pending subagent on resolution - Show a waiting indicator on non-focused confirmations Fixes QwenLM#2929
d7c92e2 to
df09141
Compare
TLDR
When multiple subagents run in parallel and each triggers a confirmation prompt, all prompts simultaneously receive keyboard focus (
isFocused={true}is hardcoded inAgentExecutionDisplay). This means pressing ↑/↓/Enter dispatches to every active confirmation at once, causing unintended approvals.This fix introduces a "first-come, first-served" focus lock so only one confirmation is interactive at a time, while others show "⏳ Waiting for other approval...".
Screenshots / Video Demo
Before (bug) — all three prompts show
›on option 2 simultaneously; pressing ↑/↓ moves the selection in all prompts at once:After (fixed) — only the first prompt is focused (
›on option 2); the other two show "⏳ Waiting for other approval..." and their selections are frozen at option 1:Dive Deeper
In
AgentExecutionDisplay.tsx,ToolConfirmationMessagewas rendered withisFocused={true}hardcoded. When multiple subagents run in parallel and each triggers a confirmation, all prompts simultaneously capture keyboard input.The fix introduces a focus lock mechanism in
ToolGroupMessage:isAgentWithPendingConfirmation): cleanly identifies subagent tool calls with pending confirmations, replacing scatteredascasts.useRef-based lock: the first subagent to present a confirmation acquires focus. The lock is released only when that confirmation is resolved, then automatically promotes to the next pending subagent.isFocusedflows downToolGroupMessage→ToolMessage→SubagentExecutionRenderer→AgentExecutionDisplay→ToolConfirmationMessage.toolAwaitingApproval) always take priority over subagent-level confirmations.isFocuseddefaults totrueinAgentExecutionDisplay, so all existing call sites are unaffected. No new dependencies introduced.Modified files:
packages/cli/src/ui/components/messages/ToolGroupMessage.tsx— Core focus-lock logic (type guard,useRef,isSubagentFocusedcomputation)packages/cli/src/ui/components/messages/ToolMessage.tsx— Prop passthrough (isFocused?: boolean)packages/cli/src/ui/components/subagents/runtime/AgentExecutionDisplay.tsx— ConsumeisFocused, render waiting indicator, forward toToolConfirmationMessageReviewer Test Plan
"Launch 3 subagents: one fetches a URL, one writes a file, one runs a shell command")›)Testing Matrix
Linked issues / bugs
Fixes #2929