fix(conditions): put a refused keystroke where a modal cannot cover it

src/providers/shared/renderer/conditions/dispatchRefusal.renderer.test.tsx

@@ -189,9 +189,10 @@ describe('the dispatcher reports what it used to discard (#1070)', () => {
   })
 
   it('leaves the pty arm alone, because its caller already answers for it', async () => {
-    // `sendConditionKey` in TileLeaf reads main's boolean and shows a pane
-    // toast for a write it could not make. Reporting here too would double the
-    // message for the one arm that was never silent.
+    // `sendConditionKey` in TileLeaf reads main's boolean and surfaces the
+    // refusal itself — on the global toast since #711, the same surface this
+    // reporter's callers use. Reporting here too would double the message for
+    // the one arm that was never silent.
     const onRefused = vi.fn()
     const onPtyAction = vi.fn(async () => {})
     const dispatch = makeOutletDispatch(onPtyAction, async () => ({ ok: false, reason: 'timeout' }), onRefused)

src/renderer/src/workspace/tile-tree/TileLeaf.conditionRefusal.renderer.test.tsx

@@ -5,6 +5,7 @@ import { useAppStore } from '@renderer/app-state/store'
 import { emptyRuntime } from '@renderer/session-runtime/state'
 import type { SessionRuntime } from '@renderer/session-runtime/state'
 import type { ConditionRefusal, ConditionRefusalReporter } from '@shared/conditions-core/dispatch'
+import type { ConditionPtyAction } from '@shared/conditions-core/contract'
 import type { Workspace } from '@renderer/workspace/hook'
 import { GlobalToastProvider } from '@renderer/ui/GlobalToast'
 import { AgentTerminalOwnerVisibilityProvider } from '@renderer/workspace/terminal/AgentTerminalOwnership'
@@ -28,15 +29,32 @@ import { TileLeaf } from './TileLeaf'
 // src/providers/shared/renderer/conditions/dispatchRefusal.renderer.test.tsx.
 
 let reporter: ConditionRefusalReporter | undefined
+/** The pty arm TileLeaf passes as `onPtyAction`, which reaches
+ *  `sendConditionKey`. Probed through the real prop (#1177 renamed it from
+ *  `onSend`), and adapted back to a keystroke so the tests read as one. A probe
+ *  that no longer matches the prop leaves `sendKey` undefined, and `mount`'s
+ *  assertion fails loudly rather than letting every keystroke test pass
+ *  vacuously. */
+let sendKey: ((data: string) => Promise<void>) | undefined
 vi.mock('@providers/shared/renderer/conditions/ProviderConditionOutlet', () => ({
-  ProviderConditionOutlet: (props: { onConditionRefused?: ConditionRefusalReporter }) => {
+  ProviderConditionOutlet: (props: {
+    onConditionRefused?: ConditionRefusalReporter
+    onPtyAction?: (action: ConditionPtyAction) => Promise<void>
+  }) => {
     reporter = props.onConditionRefused
+    const onPtyAction = props.onPtyAction
+    sendKey = onPtyAction ? data => onPtyAction({ kind: 'pty', id: 'probe', label: 'probe', data }) : undefined
     return <div data-testid="outlet" />
   },
 }))
 
 vi.mock('@renderer/features/feed/ui/Feed', () => ({ Feed: () => <div data-testid="feed" /> }))
-vi.mock('@renderer/features/sessionFeed/SessionFeedContext', () => ({ useSessionFeed: () => ({}) }))
+// `sendInput` is what main's refusal comes back through; the tests that care
+// set `sendInputResult` before clicking.
+let sendInputResult = true
+vi.mock('@renderer/features/sessionFeed/SessionFeedContext', () => ({
+  useSessionFeed: () => ({ sendInput: async () => sendInputResult }),
+}))
 vi.mock('@renderer/features/feed/ledger/useLedgerFeedItems', () => ({ useLedgerFeedItems: () => ({ items: [] }) }))
 vi.mock('@renderer/features/usage-limit/useUsageLimitActions', () => ({ useUsageLimitActions: () => ({}) }))
 vi.mock('@renderer/features/workflows/model/useSessionWorkflowViews', () => ({
@@ -54,6 +72,9 @@ const originalApi = window.api
 
 beforeEach(() => {
   reporter = undefined
+  sendKey = undefined
+  sendInputResult = true
+  paneToasts.length = 0
   window.api = {
     ...(originalApi ?? {}),
     onExtensionNotification: vi.fn().mockReturnValue(() => {}),
@@ -75,15 +96,19 @@ const refusal = (over: Partial<ConditionRefusal> = {}): ConditionRefusal => ({
   ...over,
 })
 
-function mount(): void {
+/** Every message TileLeaf tried to put in the PANE toast. */
+const paneToasts: Array<{ sessionId: string; message: string }> = []
+
+function mount(options?: { conditions?: SessionRuntime['conditions'] }): void {
   const workspace = {
     state: { sessions: { agent: { kind: 'claude', cwd: '/trial' } } },
     acknowledgeSession: vi.fn(),
     setDraftInput: vi.fn(),
+    showPaneToast: (sessionId: string, message: string) => { paneToasts.push({ sessionId, message }) },
   } as unknown as Workspace
   const runtime: SessionRuntime = {
     ...emptyRuntime(),
-    conditions: {
+    conditions: options?.conditions !== undefined ? options.conditions : {
       provider: 'claude',
       ts: 1,
       conditions: {
@@ -98,8 +123,10 @@ function mount(): void {
       </AgentTerminalOwnerVisibilityProvider>
     </GlobalToastProvider>,
   )
-  // A reporter TileLeaf never passed would make every assertion below vacuous.
+  // A reporter or send TileLeaf never passed would make every assertion below
+  // vacuous.
   expect(reporter).toBeTypeOf('function')
+  expect(sendKey).toBeTypeOf('function')
 }
 
 it('tells the user in words what the refusal was, on the surface a modal cannot cover', () => {
@@ -163,3 +190,70 @@ it('leaves the toast dismissable so it cannot sit over the question it is about'
 
   expect(screen.queryByText(message)).toBeNull()
 })
+
+// ---------------------------------------------------------------------------
+// The pty arm (#711 item 1)
+//
+// #1070 moved the STRUCTURED refusal to the global toast because every
+// condition that can refuse is a modal, and the pane toast is an in-flow
+// sibling with no z-index sitting under a 1100-z scrim. The keystroke arm was
+// left behind, so the same click produced a readable message or an invisible
+// one depending on which arm handled it — and the keystroke arm is the one a
+// TRUST DIALOG uses, i.e. the case where the modal is guaranteed to be up.
+// ---------------------------------------------------------------------------
+
+it('shows a refused keystroke where a modal cannot cover it', async () => {
+  mount()
+  sendInputResult = false
+
+  await act(async () => { await sendKey?.('1') })
+
+  const toast = screen.getByText(
+    'That keystroke did not reach the agent. If it stays stuck, retry the pane.',
+  )
+  expect(toast.closest('[class*="z-[1200]"]')).not.toBeNull()
+  // The pane toast is the surface this message used to go to, and it is under
+  // the scrim. Asserting the text moved is not enough — a message sent to BOTH
+  // would still pass that.
+  expect(paneToasts).toEqual([])
+})
+
+it('says a vanished prompt is gone on the same surface as every other refusal', async () => {
+  // The other early return of the same click. A single click must not land on
+  // two different surfaces depending on which check failed first.
+  mount({ conditions: null })
+
+  await act(async () => { await sendKey?.('1') })
+
+  expect(screen.getByText('That prompt is no longer live.')).toBeInTheDocument()
+  expect(paneToasts).toEqual([])
+})
+
+it('holds a refused keystroke as long as the refusal it matches', async () => {
+  // The comment beside the fix calls the duration load-bearing — 2.5s is not
+  // enough to look at a trust dialog and decide. A mutation that drops both
+  // duration arguments passed all 1013 renderer tests (#1110 review), because
+  // the sibling reporter's duration test was never carried across to this arm.
+  vi.useFakeTimers()
+  mount()
+  sendInputResult = false
+
+  await act(async () => { await sendKey?.('1') })
+  const message = 'That keystroke did not reach the agent. If it stays stuck, retry the pane.'
+  expect(screen.getByText(message)).toBeInTheDocument()
+
+  act(() => { vi.advanceTimersByTime(4_000) })
+  expect(screen.getByText(message)).toBeInTheDocument()
+
+  act(() => { vi.advanceTimersByTime(2_100) })
+  expect(screen.queryByText(message)).toBeNull()
+})
+
+it('stays silent when the keystroke landed', async () => {
+  mount()
+
+  await act(async () => { await sendKey?.('1') })
+
+  expect(screen.queryByText(/did not reach the agent/)).toBeNull()
+  expect(paneToasts).toEqual([])
+})

src/renderer/src/workspace/tile-tree/TileLeaf.tsx

@@ -425,25 +425,75 @@ export function TileLeaf({
   // precisely what the quarantine exists to prevent. Requiring a live
   // snapshot mirrors what `RemoteServer.applyPermissionReply` already does
   // before it writes.
+  //
+  // WHY BOTH REFUSALS GO TO THE GLOBAL TOAST (#711 item 1)
+  //
+  // This arm is reached by FOUR condition kinds, not one shape (#1110 review
+  // corrected an earlier version of this comment that claimed they were all
+  // modals):
+  //
+  //   - `claude.trust-dialog` and `claude.permission-prompt` are Radix
+  //     modals;
+  //   - `codex.approval` and `claude.resume-prompt` are in-flow STRIPS
+  //     (`layout: 'strip'`, plain `role="group"` divs, no scrim);
+  //
+  // and there is a fifth entry point with no condition surface at all:
+  // `useComposerKeybinds` routes composer ArrowUp/Down here whenever
+  // `hasActionCondition` is true.
+  //
+  // The modal cases are what force the move. `showPaneToast` renders
+  // `PaneToast`, an in-flow sibling inside the pane with no z-index; the
+  // dialog overlay is z-[1100] and 85% opaque, and Radix marks the rest of the
+  // subtree `aria-hidden`. The message was painted under the scrim, inside the
+  // region screen readers are told to ignore. Worse, a modal from a RETAINED
+  // pane (Spotlight keeps the tile tree mounted under `display: none`) portals
+  // to `body` while its pane toast is inside the hidden subtree — invisible
+  // outright. `GlobalToast` is z-[1200], and its own header records this trap.
+  //
+  // For the strip cases the pane toast was readable, and the global toast is
+  // the lesser fit: it is not adjacent to the strip that was clicked. That
+  // cost is accepted because the alternative is branching the surface on the
+  // condition's layout, which would put the same click on two surfaces again —
+  // the exact inconsistency this fixes. Every path here is focus-safe (the
+  // pane root takes `onMouseDown={onFocusRequest}`, strip keyboard handling is
+  // gated on `interactionActive`, and the composer route needs the textarea
+  // focused), so the toast always describes the pane the user is looking at.
+  //
+  // #1070 moved the STRUCTURED refusal here and left this arm behind, which
+  // was worse than either surface on its own: one click produced a readable
+  // message or an invisible one depending on which arm handled it, and this is
+  // the arm a trust dialog uses.
+  //
+  // The vanished-prompt branch moves too. It is in fact not reachable today —
+  // the outlet is fed `normalizedConditions` and renders nothing for a null
+  // snapshot, and the composer route is gated by `hasActionCondition` — so
+  // moving it costs nothing and keeps one click on one surface if it ever
+  // becomes reachable again.
+  //
+  // Duration matches the refusal reporter's: these ask the user to look at
+  // something and decide, which the 2.5s default does not allow.
   const sendConditionKey = useCallback(async (data: string) => {
     acknowledgeSession()
     if (!runtime.conditions) {
-      workspace.showPaneToast(sessionId, 'That prompt is no longer live.')
+      showToast('That prompt is no longer live.', 6000)
       return
     }
     const ok = await feed.sendInput(sessionId, data)
     if (!ok) {
+      // Parity with the structured reporter below: the toast tells the user,
+      // the console says which session, because the toast deliberately carries
+      // no session identity and a 4-pane grid gives it no context (#1110
+      // review).
+      // eslint-disable-next-line no-console
+      console.warn(`[condition ${sessionId.slice(0, 8)}] keystroke refused`)
       // main returns a bare boolean for two disjoint reasons — a prompt
       // delivery holds the write reservation, or there is no backend at all.
       // Only the first is worth retrying, and we cannot tell them apart from
       // here; the message stays honest about that rather than promising a
       // retry that can never work.
-      workspace.showPaneToast(
-        sessionId,
-        'That keystroke did not reach the agent. If it stays stuck, retry the pane.',
-      )
+      showToast('That keystroke did not reach the agent. If it stays stuck, retry the pane.', 6000)
     }
-  }, [acknowledgeSession, feed, runtime.conditions, sessionId, workspace.showPaneToast])
+  }, [acknowledgeSession, feed, runtime.conditions, sessionId, showToast])
 
   const loadOlderHistory = useCallback(async () => {
     await workspace.loadOlderHistory(sessionId)

src/shared/conditions-core/dispatch.ts

@@ -66,10 +66,11 @@ export function makeOutletDispatch(
     if (action.kind === 'pty') {
       // The pty arm is NOT reported here. Its caller owns the answer:
       // `onPtyAction` returns void because each surface already reports its
-      // own failed write (the desktop's sendConditionKey shows a pane toast
-      // for main's false; the phone shows the desktop's refusal of an action
-      // it could not match). Reporting it twice would double the message for
-      // the one arm that was never silent.
+      // own failed write (the desktop's sendConditionKey shows it on the
+      // global toast since #711, the same surface this reporter's callers
+      // use; the phone shows the desktop's refusal of an action it could not
+      // match). Reporting it twice would double the message for the one arm
+      // that was never silent.
       await onPtyAction(action)
       return
     }