fix(web-shell): make approval and question overlays keyboard accessible

packages/web-shell/client/App.test.tsx

@@ -156,6 +156,8 @@ const {
       blocks: [] as unknown[],
       messages: [] as unknown[],
       latestChatEditorProps: null as ChatEditorTestProps | null,
+      latestToolApprovalKeyboardActive: null as boolean | null,
+      latestAskUserQuestionKeyboardActive: null as boolean | null,
       latestScheduledTasksProps: null as {
         onRunPrompt?: (
           prompt: string,
@@ -766,8 +768,30 @@ mockComponent('./components/dialogs/RewindDialog', 'RewindDialog');
 mockComponent('./components/messages/AgentsMessage', 'AgentsMessage');
 mockComponent('./components/messages/MemoryMessage', 'MemoryMessage');
 mockComponent('./components/messages/AuthMessage', 'AuthMessage');
-mockComponent('./components/messages/ToolApproval', 'ToolApproval');
-mockComponent('./components/messages/AskUserQuestion', 'AskUserQuestion');
+// Record keyboardActive so app-level tests can assert the overlay is told to
+// grab focus when it becomes topmost (the actual focus lives in the real
+// components, covered by their own unit tests).
+vi.doMock('./components/messages/ToolApproval', async () => {
+  const React = await import('react');
+  return {
+    ToolApproval: (props: { keyboardActive?: boolean }) => {
+      testState.latestToolApprovalKeyboardActive = props.keyboardActive ?? null;
+      return React.createElement('div', {
+        'data-web-shell-permission-panel': '',
+      });
+    },
+  };
+});
+vi.doMock('./components/messages/AskUserQuestion', async () => {
+  const React = await import('react');
+  return {
+    AskUserQuestion: (props: { keyboardActive?: boolean }) => {
+      testState.latestAskUserQuestionKeyboardActive =
+        props.keyboardActive ?? null;
+      return React.createElement('div', { 'data-web-shell-ask-panel': '' });
+    },
+  };
+});
 mockComponent('./components/messages/TasksStatusMessage', 'TasksStatusMessage');
 mockComponent('./components/messages/BtwMessage', 'BtwMessage');
 mockComponent('./components/QueuedPromptDisplay', 'QueuedPromptDisplay');
@@ -904,6 +928,8 @@ beforeEach(() => {
   testState.blocks = [];
   testState.messages = [];
   testState.latestChatEditorProps = null;
+  testState.latestToolApprovalKeyboardActive = null;
+  testState.latestAskUserQuestionKeyboardActive = null;
   testState.latestScheduledTasksProps = null;
   testState.latestGoalsProps = null;
   sidebarTokens.length = 0;
@@ -1350,10 +1376,14 @@ describe('App session callbacks', () => {
     editorFocus.mockClear();
     act(() => vi.runOnlyPendingTimers());
 
+    // The editor isn't refocused while an approval is pending; instead the app
+    // tells the approval overlay to take focus (keyboardActive), so a stray
+    // keystroke can't send a message past the pending approval.
     expect(editorFocus).not.toHaveBeenCalled();
-    expect(document.activeElement).toBe(
+    expect(
       document.querySelector('[data-testid="approval-overlay"]'),
-    );
+    ).not.toBeNull();
+    expect(testState.latestToolApprovalKeyboardActive).toBe(true);
   });
 
   it('does not show missing-session state for non-404/410 errors', async () => {
@@ -4178,7 +4208,10 @@ describe('App session callbacks', () => {
     expect(mockSessionActions.sendPrompt).not.toHaveBeenCalled();
   });
 
-  it('moves focus to the approval overlay when it appears', async () => {
+  it('marks the approval overlay keyboard-active when it appears', async () => {
+    // Focus itself is owned by ToolApproval/AskUserQuestion (covered by their
+    // own tests); the app's job is to render the overlay and tell it to grab
+    // focus (keyboardActive) once it's the topmost surface.
     const { rerender } = renderApp();
     await flush();
 
@@ -4188,9 +4221,31 @@ describe('App session callbacks', () => {
       await Promise.resolve();
     });
 
-    const overlay = document.querySelector('[data-testid="approval-overlay"]');
-    expect(overlay).not.toBeNull();
-    expect(document.activeElement).toBe(overlay);
+    expect(
+      document.querySelector('[data-testid="approval-overlay"]'),
+    ).not.toBeNull();
+    expect(testState.latestToolApprovalKeyboardActive).toBe(true);
+  });
+
+  it('marks the ask-user question overlay keyboard-active when it appears', async () => {
+    // Symmetric to the ToolApproval case: guards against askUserOverlayVisible
+    // being mis-derived (e.g. from pendingToolApproval) so the question overlay
+    // would never pull focus.
+    const { rerender } = renderApp();
+    await flush();
+
+    await act(async () => {
+      testState.blocks = [
+        makePendingPermissionBlock({ toolName: 'ask_user_question' }),
+      ];
+      rerender();
+      await Promise.resolve();
+    });
+
+    expect(
+      document.querySelector('[data-testid="approval-overlay"]'),
+    ).not.toBeNull();
+    expect(testState.latestAskUserQuestionKeyboardActive).toBe(true);
   });
 
   it('closes the panel on Escape from outside the sidebar', async () => {

packages/web-shell/client/App.tsx

@@ -2536,30 +2536,24 @@ export function App({
     showApprovalModeDialog,
     mainView,
   ]);
-  // Once the effect above uncovers the approval, the overlay is the topmost
-  // surface but the just-unmounted panel Back button dropped focus to <body>.
-  // Move focus onto the overlay when it becomes visible so keyboard/AT users
-  // land on it. Only for ToolApproval: it drives keyboard entirely through a
-  // window listener, so focusing its (tabindex=-1) wrapper is safe and gives AT
-  // a landing spot without confirming (Enter arms first, confirms second — a
-  // focused button would confirm on the first press). AskUserQuestion instead
-  // manages its own focus across its options/input, so stealing focus to the
-  // wrapper would break its arrow-key navigation.
-  const approvalOverlayRef = useRef<HTMLDivElement | null>(null);
+  // Whether each approval overlay is the topmost (visible, uncovered) one. The
+  // overlay components consume this as `keyboardActive`: when it flips true — on
+  // appearance, or once a panel/dialog that was covering it closes — they pull
+  // keyboard focus to their own safe-default option. Focus handling now lives in
+  // ToolApproval/AskUserQuestion (their keyboard handling is focus-scoped), so
+  // the app no longer focuses the wrapper element directly.
   const toolApprovalOverlayVisible =
     pendingToolApproval !== null &&
     !activePanel &&
     modelDialogMode === null &&
     !showApprovalModeDialog &&
     mainView === 'chat';
-  const prevToolApprovalOverlayVisibleRef = useRef(toolApprovalOverlayVisible);
-  useEffect(() => {
-    const wasVisible = prevToolApprovalOverlayVisibleRef.current;
-    prevToolApprovalOverlayVisibleRef.current = toolApprovalOverlayVisible;
-    if (toolApprovalOverlayVisible && !wasVisible) {
-      approvalOverlayRef.current?.focus();
-    }
-  }, [toolApprovalOverlayVisible]);
+  const askUserOverlayVisible =
+    pendingAskUserApproval !== null &&
+    !activePanel &&
+    modelDialogMode === null &&
+    !showApprovalModeDialog &&
+    mainView === 'chat';
   const [showMemoryDialog, setShowMemoryDialog] = useState(false);
   const [showAuthDialog, setShowAuthDialog] = useState(false);
   const showAuthDialogRef = useRef(showAuthDialog);
@@ -7061,34 +7055,32 @@ export function App({
                       )}
                       {/* Only render the outer session's approval on the chat
                           view. Under a full-page view (split / scheduled tasks)
-                          it would sit hidden yet still own global keyboard
-                          shortcuts — a keypress could confirm an unseen
-                          approval. Each split pane surfaces its own approval. */}
+                          it would sit hidden and unreachable. Each split pane
+                          surfaces its own approval. `keyboardActive` tells the
+                          overlay to grab focus only when it's the topmost one. */}
                       {pendingToolApproval && mainView === 'chat' && (
                         <div
-                          ref={approvalOverlayRef}
-                          tabIndex={-1}
                           data-testid="approval-overlay"
                           className={styles.approvalOverlay}
                         >
                           <ToolApproval
                             request={pendingToolApproval}
                             onConfirm={handleConfirm}
                             variant="floating"
+                            keyboardActive={toolApprovalOverlayVisible}
                           />
                         </div>
                       )}
                       {pendingAskUserApproval && mainView === 'chat' && (
                         <div
-                          ref={approvalOverlayRef}
-                          tabIndex={-1}
                           data-testid="approval-overlay"
                           className={styles.approvalOverlay}
                         >
                           <AskUserQuestion
                             request={pendingAskUserApproval}
                             onConfirm={handleConfirm}
                             variant="floating"
+                            keyboardActive={askUserOverlayVisible}
                           />
                         </div>
                       )}

packages/web-shell/client/components/ChatPane.test.tsx

@@ -205,6 +205,7 @@ vi.mock('./messages/AskUserQuestion', () => ({
   AskUserQuestion: (props: any) => (
     <button
       data-testid="ask-approval"
+      data-keyboard-active={String(props.keyboardActive)}
       onClick={() => props.onConfirm(props.request.id, 'opt')}
     >
       ask
@@ -581,6 +582,12 @@ describe('ChatPane', () => {
     };
     render();
     expect(testid('ask-approval')).not.toBeNull();
+    // Like the tool-approval path, a pane's question must not auto-grab focus —
+    // several panes can show at once and stealing focus would yank it from the
+    // pane the user is in.
+    expect(testid('ask-approval')?.getAttribute('data-keyboard-active')).toBe(
+      'false',
+    );
     // AskUserQuestion is not a tool approval, so MessageList gets no inline one.
     expect(testid('pane-messages')?.getAttribute('data-approval')).toBe('no');
   });

packages/web-shell/client/components/ChatPane.tsx

@@ -556,9 +556,10 @@ export function ChatPane({
               request={pendingToolApproval}
               onConfirm={handleConfirm}
               variant="floating"
-              // Several panes can show approvals at once; global Enter/Escape
-              // shortcuts aren't focus-scoped, so keep pane approvals
-              // click-only to avoid confirming the wrong session's request.
+              // Several panes can show approvals at once; don't auto-focus one
+              // pane's approval (it would steal focus from the pane the user is
+              // in). Keyboard handling is focus-scoped, so each pane's approval
+              // is still fully keyboard-operable once clicked/tabbed into.
               keyboardActive={false}
             />
           </div>
@@ -569,6 +570,7 @@ export function ChatPane({
               request={pendingAskUserApproval}
               onConfirm={handleConfirm}
               variant="floating"
+              keyboardActive={false}
             />
           </div>
         )}

packages/web-shell/client/components/messages/AskUserQuestion.module.css

@@ -166,14 +166,29 @@
   display: flex;
   align-items: flex-start;
   gap: 10px;
+  width: 100%;
   min-height: 36px;
   padding: 7px 12px;
+  border: 0;
   border-radius: 12px;
+  background: transparent;
   color: var(--foreground);
   cursor: pointer;
+  font-family: var(--font-sans, system-ui, sans-serif);
   font-size: 14px;
   font-weight: 500;
   line-height: 22px;
+  text-align: left;
+  appearance: none;
+}
+
+.option:hover {
+  background: var(--accent);
+}
+
+.option:focus-visible {
+  outline: 2px solid var(--agent-blue-500);
+  outline-offset: -2px;
 }
 
 .optionActive,
@@ -281,6 +296,30 @@
   outline: none;
 }
 
+/* The "Other" row's clickable label when not editing — a real <button> so it's
+   keyboard-reachable, styled to read like the option label it replaces. */
+.customTrigger {
+  flex: 1;
+  min-width: 0;
+  padding: 0;
+  border: 0;
+  border-radius: 4px;
+  background: transparent;
+  color: var(--foreground);
+  font-family: var(--font-sans, system-ui, sans-serif);
+  font-size: 14px;
+  font-weight: 500;
+  line-height: 22px;
+  text-align: left;
+  cursor: pointer;
+  appearance: none;
+}
+
+.customTrigger:focus-visible {
+  outline: 2px solid var(--agent-blue-500);
+  outline-offset: 2px;
+}
+
 .customInput:focus {
   border-bottom-color: var(--agent-blue-500);
 }